Skip to content
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

feat: TypeScript 5.7 #27857

Merged
merged 32 commits into from
Jan 31, 2025
Merged

feat: TypeScript 5.7 #27857

merged 32 commits into from
Jan 31, 2025

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Jan 29, 2025

cli/build.rs Show resolved Hide resolved
@dsherret dsherret marked this pull request as ready for review January 31, 2025 16:47
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the lib.dom types haven't been updated to use typed arrays. This is going to be a painful transition, but I think it's probably best for us to keep aligning with TypeScript.

cli/build.rs Outdated Show resolved Hide resolved
Comment on lines +803 to +808
const setTypesNodeIgnorableNames = new Set([
"AbortController",
"AbortSignal",
"AsyncIteratorObject",
"atob",
"Blob",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you generate that list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added logging for whenever the @types/node project assigned to a global then put everything in here then deleted the ones I didn't think were necessary. Probably some of these could still be removed. It actually might be better for us to use an allow list, but I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more worried about getting this list again in X months.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see how this goes. I mostly just kept it to anything that was causing errors with our globals so maybe we can remove or add to this over time based on feedback and errors we encounter when upgrading.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was lazy :)

I trimmed these out when updating the local @types/node package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, seems like a lot of noise

Copy link
Member

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dsherret dsherret merged commit 8971064 into denoland:main Jan 31, 2025
18 checks passed
@dsherret dsherret deleted the typescript_5_7 branch January 31, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to TypeScript 5.7
5 participants