-
Notifications
You must be signed in to change notification settings - Fork 214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Typescript 5.6 #9795
Typescript 5.6 #9795
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
6261e63
to
3706d47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that 5.6 went final last night, so let's switch to that instead of the rc. It also seems that they may have reverted some changes that caused some of the Node and Iterator types compatibility issues that had to be worked around here, so let's possibly consider stashing those for now.
packages/internal/src/utils.js
Outdated
[Symbol.asyncDispose]() { | ||
throw Error('asyncDispose not implemented.'); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the proposal, on async iterators this is the equivalent of calling return()
.
[Symbol.asyncDispose]() { | |
throw Error('asyncDispose not implemented.'); | |
}, | |
async [Symbol.asyncDispose]() { | |
await reader.return(); | |
}, |
e119569
to
7da7902
Compare
As a matter of process, it's really difficult to review what changed from my previous review when the base commit this PR is staged on changed since the comparison of changes since last review is now broken. In general while the review is in progress, prefer merging changes from master if you must update the PR so that changes remain additive.
Have we tried reverting the version bump of |
I'll leave process discussions to internal forums.
Reverting the commit cause dozens of errors, all with the Buffer type due to IterableIterator types:
We run CI tests with Node 18 so the only risk of Node 22 types is incorrect static analysis. |
Ah got it, it's due to Iterator helpers addition, not the other iterable object type change. The problem is most likely because we target
Of course, my goal was to keep the tools working as best as possible, and the development iteration short. |
dcf506f
to
3f872ab
Compare
3f872ab
to
3a1d360
Compare
the `import *` is no longer working for some reason
77c92f9
to
37e43c6
Compare
Bypassed integration because this PR has passed integration several times save for multichain-testing, and I just reverted the changes to that. |
evergreen
Description
https://devblogs.microsoft.com/typescript/announcing-typescript-5-6
Notable changes for this repo,
Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
none
Testing Considerations
I captured type coverage before and after.
Upgrade Considerations
n/a