-
Notifications
You must be signed in to change notification settings - Fork 642
Port node18
and nodenext
changes
#1070
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
Conversation
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.
Pull Request Overview
This PR ports changes from upstream TypeScript pull requests to add support for Node18 alongside node16 and nodenext. Key changes include:
- Updated baselines and type declarations for ESM modules and dynamic imports.
- Inclusion of Node18 in module resolution and compiler options.
- Addition of a new helper method (GetEmitSyntaxForUsageLocation) and updated grammar checks in the checker.
Reviewed Changes
Copilot reviewed 379 out of 379 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
testdata/baselines/reference/submodule/compiler/esmNoSynthesizedDefault* | Updated error, type, and symbol baselines reflecting new default export behavior for ESM modules |
testdata/baselines/reference/submodule/compiler/esmModeDeclarationFileWithExportAssignment* | Adjustments to type and error baselines for export assignments under ESM contexts |
testdata/baselines/reference/submodule/compiler/dynamicImportsDeclaration* | Revised dynamic import declarations to output default export unions consistently |
internal/core/compileroptions.go | Revised module kind switch cases to include Node18 contexts for script target and module resolution |
internal/compiler/program.go | Added GetEmitSyntaxForUsageLocation to support dynamic import syntax adjustments |
internal/compiler/fileloader.go | Refactored usage of fileEmitMode to enhance clarity in dynamic import handling |
internal/checker/*.go | Updated grammar checks and module kind comparisons to properly include Node18 ranges |
Comments suppressed due to low confidence (2)
internal/core/compileroptions.go:152
- Consider adding a clarifying comment that Node18 is intentionally included alongside Node16 in GetEmitScriptTarget and GetModuleResolutionKind to aid future maintainers.
case ModuleKindNode16, ModuleKindNodeNext:
internal/checker/checker.go:5009
- Review the updated condition to ensure that extending the module kind range to include Node18 accurately reflects the intended behavior for JSON import checks.
if !importClause.IsTypeOnly() && core.ModuleKindNode18 <= c.moduleKind && c.moduleKind <= core.ModuleKindNodeNext && c.isOnlyImportableAsDefault(moduleSpecifier, resolvedModule) && !hasTypeJsonImportAttribute(node) {
internal/checker/checker.go
Outdated
// return host.getEmitSyntaxForUsageLocation(ast.GetSourceFileOfNode(usage), usage) | ||
// } | ||
if ast.IsStringLiteralLike(usage) { | ||
return c.program.GetEmitSyntaxForUsageLocation(ast.GetSourceFileOfNode(usage), usage) |
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.
This was not part of the PRs but interacted with diffs they changed; I missed it before. There's also another bit in resolveESModuleSymbol
I'm going to port in a separate PR.
Tests fail because |
I hate that test |
Ports
--module node18
TypeScript#60722require(esm)
in--module nodenext
TypeScript#60761