fix(api): Activity.timestamp should be string instead of Date (type only)#571
Open
heyitsaamir wants to merge 1 commit into
Open
fix(api): Activity.timestamp should be string instead of Date (type only)#571heyitsaamir wants to merge 1 commit into
heyitsaamir wants to merge 1 commit into
Conversation
The Bot Framework Activity protocol sends these as ISO-8601 strings over JSON (as the existing doc comments already say). Inbound activities are parsed by express.json() with no reviver and Activity.from() is just Object.assign — so at runtime these fields are strings, not Date objects. The Date type was a lie; calling activity.timestamp.toISOString() in a handler would crash. The withTimestamp/withLocalTimestamp builders still accept Date for ergonomics and normalize via toISOString(), so existing call sites keep working.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR corrects the public typings for Activity.timestamp and Activity.localTimestamp in @microsoft/teams.api to reflect actual runtime behavior: these fields are ISO-8601 strings on inbound activities, not Date objects. It also updates the fluent builder methods to keep accepting Date inputs while normalizing them to ISO strings to avoid breaking common construction patterns.
Changes:
- Change
IActivity.timestamp/localTimestampandActivity.timestamp/localTimestampfromDatetostring. - Update
withTimestampandwithLocalTimestampto acceptDate | string, convertingDateinputs totoISOString().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
49
to
+53
| * Contains the local date and time of the message, expressed in ISO-8601 format. | ||
| * | ||
| * For example, 2016-09-23T13:07:49.4714686-07:00. | ||
| */ | ||
| localTimestamp?: Date; | ||
| localTimestamp?: string; |
Comment on lines
+305
to
307
| withTimestamp(value: Date | string) { | ||
| this.timestamp = value instanceof Date ? value.toISOString() : value; | ||
| return this; |
Comment on lines
+315
to
317
| withLocalTimestamp(value: Date | string) { | ||
| this.localTimestamp = value instanceof Date ? value.toISOString() : value; | ||
| return this; |
heyitsaamir
changed the title
rido-min
reviewed
May 11, 2026
|
|
||
| withTimestamp(value: Date) { | ||
| this.timestamp = value; | ||
| withTimestamp(value: Date | string) { |
Member
There was a problem hiding this comment.
we should review if this method should be marked as obsolete, along with changes in #525. Timestamps set on outgoing activities are ignored
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Activity.timestampwas lying about being aDate. It's a string. Always was. The doc comment literally says "expressed in ISO-8601 format" two lines above the: Dateannotation 🙃Inbound activities go through
express.json()(no reviver), andActivity.from()is justObject.assign— nothing ever turns it into aDate. Soactivity.timestamp.toISOString()in a handler? Crash.Fix: type as
string. Builders still acceptDateand normalize viatoISOString()so nobody's call sites blow up.Same treatment for
localTimestamp.