Conversation
typescript-bot
added
the
For Uncommitted Bug
Renegade334
force-pushed
the
lib-esnext-temporal
branch
2 times, most recently
from
f8fc33a to
9f45da2
Compare
|
We usually don't create tests for Probably we should just delete them? @typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
Dealer's choice. I needed them for testing anyway, but if they're of no value within the library then they can just go. |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Well... type DateLikeObjectBase = {
year?: number | undefined;
era?: string | undefined;
eraYear?: number | undefined;
month?: number | undefined;
monthCode?: string | undefined;
day: number;
calendar?: string | undefined;
}
type DateLikeObject = DateLikeObjectBase & ({ month: number } | { monthCode: string });Possibly it's not worth the additional complexity, though. |
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
Renegade334
force-pushed
the
lib-esnext-temporal
branch
from
ef570f6 to
bab5a4a
Compare
|
I know there are existing polyfills for Temporal out there in TS; are these types compatible / based off of those? Or fresh? |
|
The definitions are de novo, so no licensing issues. As for compatibility:
|
- Replace custom TemporalDateType interface with union type of native Temporal.PlainDate | Temporal.PlainDateTime | Temporal.ZonedDateTime - Update temporal.d.ts to match latest TC39 spec (no Calendar class) - Adapter now uses global Temporal namespace without polyfill dependency - Polyfill (@js-temporal/polyfill) is only used in tests This prepares for when TypeScript adds lib.esnext.temporal support. See: microsoft/TypeScript#62628
- Replace custom TemporalDateType interface with union type of native Temporal.PlainDate | Temporal.PlainDateTime | Temporal.ZonedDateTime - Update temporal.d.ts to match TC39 spec and TypeScript PR #62628 (microsoft/TypeScript#62628) - Types match MDN documentation and Chrome 144/Firefox 139 implementations - Add _invalid marker to sentinel object for explicit invalid detection - Adapter uses global Temporal namespace without polyfill dependency - Polyfill (@js-temporal/polyfill) is only used in tests
|
I'm reviewing this PR now; wish me luck...
MDN I guess hasn't gotten the memo? |
|
One thing this PR is missing are updates to |
Renegade334
force-pushed
the
lib-esnext-temporal
branch
from
90da510 to
e98ac11
Compare
Renegade334
force-pushed
the
lib-esnext-temporal
branch
2 times, most recently
from
ba7824e to
a5ad09f
Compare
Renegade334
force-pushed
the
lib-esnext-temporal
branch
from
a5ad09f to
fdb0bc2
Compare
|
A few things:
|
I couldn't get this to work, at least not when the Temporal namespace itself is missing. If anyone else wants to have a look, feel free. |
| nanoseconds?: number | undefined; | ||
| } | ||
|
|
||
| interface MonthDayLikeObject extends Omit<DateLikeObject, "era" | "eraYear"> {} |
There was a problem hiding this comment.
I believe that this Omit is unnecessary because an era / eraYear pair is always accepted anywhere a year is also accepted. Here's an example from Chrome 144:
Temporal.PlainMonthDay.from({day: 1, month: 1, era: 'ad', eraYear: 2026, calendar: 'gregory'})
// => PlainMonthDay {calendarId: 'gregory', monthCode: 'M01', day: 1}
Temporal.PlainMonthDay.from({day: 1, month: 1, era: 'ad', eraYear: 2026, calendar: 'iso8601'})
// => PlainMonthDay {calendarId: 'iso8601', monthCode: 'M01', day: 1}
Temporal.PlainMonthDay.from({day: 1, month: 1, era: 'ad', eraYear: 2026})
// => PlainMonthDay {calendarId: 'iso8601', monthCode: 'M01', day: 1}FYI @sffc @ptomato in case you want to weigh in on the differences between ECMA-262 and ECMA-402 around eras.
|
Thanks for landing Temporal types in TS! I wrote the original type declarations for the Temporal proposal many years ago, and it's great to see how the pros do it. Sorry to find this PR too late (I saw it linked from Rob Palmer's tweet today) before merging. I did a quick review tonight and didn't see any obvious problems except I left one comment about what looks like a small mistake in one type.
Is there a reason why @bakkot's suggestion above (from #62628 (comment)) didn't work? Any date-like object in Temporal requires either
FYI @arshaw (maintainer of @Renegade334 @jakebailey What would you recommend that we should do in the polyfills we maintain so that polyfills are maximally compatible with TS? |
|
Hey @justingrant (and other friends) thanks for taking a look - and of course for the work you've done on Temporal! People are largely out for the weekend, but we're open to making things more compatible where it makes sense. I think in some ways this kind of raises a thought I had of whether it might have made sense for one polyfill's declaration files to be contributed as part of On the other hand, for a variety of reasons, We are planning to go out with the beta pretty soon, and I don't think these are necessarily blockers for shipping 6.0 beta with temporal. We can take some time to iron out the differences after that. Our nightlies tend to have more usage than our beta releases (plus we have an RC release), so I think we'll still get decent feedback there. |
|
👋!
Just some context from me, to expand on Daniel's comment. Dictionaries as There are fudges, but none of them offer perfect forward extensibility.
Yes, this is precaution around a scenario that doesn't yet exist! I'm not opposed to different approaches, but the one used currently is the one consistent with lib.d.ts as a whole, for those reasons. |
|
BTW, we're all so happy that TS is integrating Temporal! Didn't mean anything above to come off as criticism. Y'all are doing great work.
There's different conventions in Also, some proposals now include TS types as part of the proposal itself. In Temporal's case, we built TS types as part of the non-production polyfill we use to run tests. The production polyfill Even if you don't re-use anything (which is also fine, I don't think anyone is too picky about this), IMO a good habit would be to tag proposal champions when building the types so we could review and, if needed, comment about usage experience, future directions, etc. as you prioritize various concerns & tradeoffs.
Yeah, it's a hard problem with no perfect solution. Although I do have two suggestions: The first is that But today each linter needs to decide for themselves what beyond- The second idea is that this problem is not unique to Temporal. I've faced the "interface declaration merging cannot remove members or overloads" many times before, and it's extremely frustrating. For example, I recently had to add type safety to an app using Vuex 3.x. What we wanted to do was to replace Vuex's If there were a way for a later TS version to allow interface declarations to disable a member of that interface declared elsewhere, then it would in theory solve the forward-compatibility problem and many others. A naive solution could be something like this: // foo2026.d.ts
interface Foo {
bar(something: FooBar1): number;
}
// foo2028.d.ts
interface Foo {
// invalidate member declarations that match exactly
delete bar(something: FooBar1): number;
// optionally replace the deleted member with a newer version.
bar(something: FooBar2): number;
}I'm not familiar enough to know if something like this would be workable, but satisfying the underlying requirement to disable merged interface members would be quite helpful! |
Oh one more thing: where in MDN are you seeing content about custom calendars? We'll want to let them know to remove it. @jakebailey |
This is my bad; I confused custom calendars with non-Gregorian calendars. |
8 participants
Closes #60164.
No custom calendar support, as per the latest spec changes.
There are lots of places in the spec where property bags have "at least one of" constraints (eg. either
monthormonthCodeis mandatory for a DateLikeObject), which can't really be straightforwardly expressed. Otherwise, appears to be playing well.Temporal shipped in Firefox 139, and is staging in Chromium (shipping in 144).