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.

Sign up for GitHub

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

Add support for import defer proposal #60757

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for import defer proposal #60757

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de82ce7
Add support for `import defer` proposal
ryzokuken Jul 8, 2024
9e43939
Update AST to unify `type` with other modifiers
nicolo-ribaudo Dec 19, 2024
b1f506b
Add test for comments around `defer`
nicolo-ribaudo Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ import {
ImportDeclaration,
ImportEqualsDeclaration,
ImportOrExportSpecifier,
ImportPhase,
ImportSpecifier,
ImportTypeNode,
IndexedAccessType,
Expand Down Expand Up @@ -9858,6 +9859,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
propertyName && isIdentifier(propertyName) ? factory.createIdentifier(idText(propertyName)) : undefined,
factory.createIdentifier(localName),
)]),
ImportPhase.Evaluation,
),
factory.createStringLiteral(specifier),
/*attributes*/ undefined,
Expand Down Expand Up @@ -9944,7 +9946,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
addResult(
factory.createImportDeclaration(
/*modifiers*/ undefined,
factory.createImportClause(isTypeOnly, factory.createIdentifier(localName), /*namedBindings*/ undefined),
factory.createImportClause(isTypeOnly, factory.createIdentifier(localName), /*namedBindings*/ undefined, ImportPhase.Evaluation),
specifier,
attributes,
),
Expand All @@ -9959,7 +9961,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
addResult(
factory.createImportDeclaration(
/*modifiers*/ undefined,
factory.createImportClause(isTypeOnly, /*name*/ undefined, factory.createNamespaceImport(factory.createIdentifier(localName))),
factory.createImportClause(isTypeOnly, /*name*/ undefined, factory.createNamespaceImport(factory.createIdentifier(localName)), ImportPhase.Evaluation),
specifier,
(node as ImportClause).parent.attributes,
),
Expand Down Expand Up @@ -9995,6 +9997,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
factory.createIdentifier(localName),
),
]),
ImportPhase.Evaluation,
),
specifier,
(node as ImportSpecifier).parent.parent.parent.attributes,
Expand Down Expand Up @@ -52860,6 +52863,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (node.isTypeOnly && node.namedBindings?.kind === SyntaxKind.NamedImports) {
return checkGrammarNamedImportsOrExports(node.namedBindings);
}
if (node.phase !== ImportPhase.Evaluation && moduleKind !== ModuleKind.ESNext) {
return grammarErrorOnNode(node, Diagnostics.Deferred_imports_are_only_supported_when_the_module_flag_is_set_to_esnext);
}
return false;
}

Expand Down
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8433,5 +8433,17 @@
"String literal import and export names are not supported when the '--module' flag is set to 'es2015' or 'es2020'.": {
"category": "Error",
"code": 18057
},
"Default imports aren't allowed for deferred imports.": {
"category": "Error",
"code": 18058
},
"Named imports aren't allowed for deferred imports.": {
"category": "Error",
"code": 18059
},
"Deferred imports are only supported when the '--module' flag is set to 'esnext'.": {
"category": "Error",
"code": 18060
}
}
5 changes: 5 additions & 0 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ import {
ImportDeclaration,
ImportEqualsDeclaration,
ImportOrExportSpecifier,
ImportPhase,
ImportSpecifier,
ImportTypeNode,
IndexedAccessTypeNode,
Expand Down Expand Up @@ -3689,6 +3690,10 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
emitTokenWithComment(SyntaxKind.TypeKeyword, node.pos, writeKeyword, node);
writeSpace();
}
else if (node.phase !== ImportPhase.Evaluation) {
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved
emitTokenWithComment(SyntaxKind.DeferKeyword, node.pos, writeKeyword, node);
writeSpace();
}
emit(node.name);
if (node.name && node.namedBindings) {
emitTokenWithComment(SyntaxKind.CommaToken, node.name.end, writePunctuation, node);
Expand Down
9 changes: 6 additions & 3 deletions src/compiler/factory/nodeFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ import {
ImportClause,
ImportDeclaration,
ImportEqualsDeclaration,
ImportPhase,
ImportSpecifier,
ImportTypeAssertionContainer,
ImportTypeNode,
Expand Down Expand Up @@ -4723,11 +4724,12 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function createImportClause(isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause {
function createImportClause(isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined, phase: ImportPhase): ImportClause {
Copy link
Member

Choose a reason for hiding this comment

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

Preemptively noting that this is a breaking API change, and would require a deprecation helper in the deprecations project to tack onto our public API something that will set a default for the new parameter.

Copy link
Member

Choose a reason for hiding this comment

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

It may also be the case that phase needs to go after isTypeOnly since AST node builder parameters and properties are intended to be in source order.

Copy link
Member

@rbuckton rbuckton Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we want to mix type imports with defer or source. Something like import type source foo from "foo" doesn't make sense if all source imports will have the same type, and import type defer * as foo from "foo" would essentially be the same as import type * as foo from "foo".

If we consider type, defer, and source to be mutually exclusive, then I would suggest we replace the isTypeOnly parameter with something like importModifier: SyntaxKind.TypeKeyword | SyntaxKind.DeferKeyword | SyntaxKind.SourceKeyword | boolean | undefined and have ImportClause be:

export interface ImportClause extends NamedDeclaration {
    readonly kind: SyntaxKind.ImportClause;
    readonly parent: ImportDeclaration | JSDocImportTag;
    /** @deprecated */
    readonly isTypeOnly: boolean;
    readonly importModifier: SyntaxKind.TypeKeyword | SyntaxKind.DeferKeyword | SyntaxKind.SourceKeyword | undefined;
    readonly name?: Identifier; // Default binding
    readonly namedBindings?: NamedImportBindings;
}

For back-compat purposes, we can set isTypeOnly to true if importModifier is SyntaxKind.TypeKeyword.

Copy link
Member

Choose a reason for hiding this comment

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

I think the issue with that scheme is that it's a little weirder to capture multiple modifiers being used in the cases of error recovery.

Copy link
Author

Choose a reason for hiding this comment

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

I was originally going with something very similar to Ron's suggestion, and consider "type" just as another phase. I ended up not doing it because of the AST breaking change, but if just keeping the old property for backwards compat is ok then I'd go for it.

I think the issue with that scheme is that it's a little weirder to capture multiple modifiers being used in the cases of error recovery.

Error recovery is going to be tricky anyway, because each modifier is a valid identifier so things like import type defer are a potentially valid start of an import declaration. I wonder how likely it is for people to accidentally write two modifiers in an import.

const node = createBaseDeclaration<ImportClause>(SyntaxKind.ImportClause);
node.isTypeOnly = isTypeOnly;
node.name = name;
node.namedBindings = namedBindings;
node.phase = phase;
node.transformFlags |= propagateChildFlags(node.name) |
propagateChildFlags(node.namedBindings);
if (isTypeOnly) {
Expand All @@ -4738,11 +4740,12 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function updateImportClause(node: ImportClause, isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined) {
function updateImportClause(node: ImportClause, isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined, phase: ImportPhase) {
return node.isTypeOnly !== isTypeOnly
|| node.name !== name
|| node.namedBindings !== namedBindings
? update(createImportClause(isTypeOnly, name, namedBindings), node)
|| node.phase !== phase
? update(createImportClause(isTypeOnly, name, namedBindings, phase), node)
: node;
}

Expand Down
3 changes: 2 additions & 1 deletion src/compiler/factory/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import {
ImportCall,
ImportDeclaration,
ImportEqualsDeclaration,
ImportPhase,
InternalEmitFlags,
isAssignmentExpression,
isAssignmentOperator,
Expand Down Expand Up @@ -730,7 +731,7 @@ export function createExternalHelpersImportDeclarationIfNeeded(nodeFactory: Node

const externalHelpersImportDeclaration = nodeFactory.createImportDeclaration(
/*modifiers*/ undefined,
nodeFactory.createImportClause(/*isTypeOnly*/ false, /*name*/ undefined, namedBindings),
nodeFactory.createImportClause(/*isTypeOnly*/ false, /*name*/ undefined, namedBindings, ImportPhase.Evaluation),
nodeFactory.createStringLiteral(externalHelpersModuleNameText),
/*attributes*/ undefined,
);
Expand Down
38 changes: 29 additions & 9 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ import {
ImportDeclaration,
ImportEqualsDeclaration,
ImportOrExportSpecifier,
ImportPhase,
ImportSpecifier,
ImportTypeAssertionContainer,
ImportTypeNode,
Expand Down Expand Up @@ -7190,6 +7191,7 @@ namespace Parser {
// could be legal, it would add complexity for very little gain.
case SyntaxKind.InterfaceKeyword:
case SyntaxKind.TypeKeyword:
case SyntaxKind.DeferKeyword:
return nextTokenIsIdentifierOnSameLine();
case SyntaxKind.ModuleKeyword:
case SyntaxKind.NamespaceKeyword:
Expand Down Expand Up @@ -7221,7 +7223,7 @@ namespace Parser {

case SyntaxKind.ImportKeyword:
nextToken();
return token() === SyntaxKind.StringLiteral || token() === SyntaxKind.AsteriskToken ||
return token() === SyntaxKind.DeferKeyword || token() === SyntaxKind.StringLiteral || token() === SyntaxKind.AsteriskToken ||
token() === SyntaxKind.OpenBraceToken || tokenIsIdentifierOrKeyword(token());
case SyntaxKind.ExportKeyword:
let currentToken = nextToken();
Expand Down Expand Up @@ -7295,6 +7297,7 @@ namespace Parser {
case SyntaxKind.NamespaceKeyword:
case SyntaxKind.TypeKeyword:
case SyntaxKind.GlobalKeyword:
case SyntaxKind.DeferKeyword:
// When these don't start a declaration, they're an identifier in an expression statement
return true;

Expand Down Expand Up @@ -8366,6 +8369,7 @@ namespace Parser {
}

let isTypeOnly = false;
let phase = ImportPhase.Evaluation;
if (
identifier?.escapedText === "type" &&
(token() !== SyntaxKind.FromKeyword || isIdentifier() && lookAhead(nextTokenIsFromKeywordOrEqualsToken)) &&
Expand All @@ -8374,12 +8378,20 @@ namespace Parser {
isTypeOnly = true;
identifier = isIdentifier() ? parseIdentifier() : undefined;
}
else if (identifier?.escapedText === "defer" && token() !== SyntaxKind.FromKeyword) {
phase = ImportPhase.Defer;
identifier = undefined;
if (isIdentifier()) {
parseErrorAtCurrentToken(Diagnostics.Default_imports_aren_t_allowed_for_deferred_imports);
identifier = parseIdentifier();
}
}

if (identifier && !tokenAfterImportedIdentifierDefinitelyProducesImportDeclaration()) {
if (identifier && !tokenAfterImportedIdentifierDefinitelyProducesImportDeclaration() && phase !== ImportPhase.Defer) {
return parseImportEqualsDeclaration(pos, hasJSDoc, modifiers, identifier, isTypeOnly);
}

const importClause = tryParseImportClause(identifier, afterImportPos, isTypeOnly);
const importClause = tryParseImportClause(identifier, afterImportPos, isTypeOnly, /*skipJsDocLeadingAsterisks*/ undefined, phase);
const moduleSpecifier = parseModuleSpecifier();
const attributes = tryParseImportAttributes();

Expand All @@ -8388,7 +8400,7 @@ namespace Parser {
return withJSDoc(finishNode(node, pos), hasJSDoc);
}

function tryParseImportClause(identifier: Identifier | undefined, pos: number, isTypeOnly: boolean, skipJsDocLeadingAsterisks = false) {
function tryParseImportClause(identifier: Identifier | undefined, pos: number, isTypeOnly: boolean, skipJsDocLeadingAsterisks = false, phase: ImportPhase) {
// ImportDeclaration:
// import ImportClause from ModuleSpecifier ;
// import ModuleSpecifier;
Expand All @@ -8398,7 +8410,7 @@ namespace Parser {
token() === SyntaxKind.AsteriskToken || // import *
token() === SyntaxKind.OpenBraceToken // import {
) {
importClause = parseImportClause(identifier, pos, isTypeOnly, skipJsDocLeadingAsterisks);
importClause = parseImportClause(identifier, pos, isTypeOnly, skipJsDocLeadingAsterisks, phase);
parseExpected(SyntaxKind.FromKeyword);
}
return importClause;
Expand Down Expand Up @@ -8464,7 +8476,7 @@ namespace Parser {
return finished;
}

function parseImportClause(identifier: Identifier | undefined, pos: number, isTypeOnly: boolean, skipJsDocLeadingAsterisks: boolean) {
function parseImportClause(identifier: Identifier | undefined, pos: number, isTypeOnly: boolean, skipJsDocLeadingAsterisks: boolean, phase: ImportPhase) {
// ImportClause:
// ImportedDefaultBinding
// NameSpaceImport
Expand All @@ -8480,11 +8492,19 @@ namespace Parser {
parseOptional(SyntaxKind.CommaToken)
) {
if (skipJsDocLeadingAsterisks) scanner.setSkipJsDocLeadingAsterisks(true);
namedBindings = token() === SyntaxKind.AsteriskToken ? parseNamespaceImport() : parseNamedImportsOrExports(SyntaxKind.NamedImports);
if (token() === SyntaxKind.AsteriskToken) {
namedBindings = parseNamespaceImport();
}
else {
if (phase === ImportPhase.Defer) {
parseErrorAtCurrentToken(Diagnostics.Named_imports_aren_t_allowed_for_deferred_imports);
}
namedBindings = parseNamedImportsOrExports(SyntaxKind.NamedImports);
}
if (skipJsDocLeadingAsterisks) scanner.setSkipJsDocLeadingAsterisks(false);
}

return finishNode(factory.createImportClause(isTypeOnly, identifier, namedBindings), pos);
return finishNode(factory.createImportClause(isTypeOnly, identifier, namedBindings, phase), pos);
}

function parseModuleReference() {
Expand Down Expand Up @@ -9518,7 +9538,7 @@ namespace Parser {
identifier = parseIdentifier();
}

const importClause = tryParseImportClause(identifier, afterImportTagPos, /*isTypeOnly*/ true, /*skipJsDocLeadingAsterisks*/ true);
const importClause = tryParseImportClause(identifier, afterImportTagPos, /*isTypeOnly*/ true, /*skipJsDocLeadingAsterisks*/ true, ImportPhase.Evaluation);
const moduleSpecifier = parseModuleSpecifier();
const attributes = tryParseImportAttributes();

Expand Down
1 change: 1 addition & 0 deletions src/compiler/scanner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ export const textToKeywordObj: MapLike<KeywordSyntaxKind> = {
true: SyntaxKind.TrueKeyword,
try: SyntaxKind.TryKeyword,
type: SyntaxKind.TypeKeyword,
defer: SyntaxKind.DeferKeyword,
typeof: SyntaxKind.TypeOfKeyword,
undefined: SyntaxKind.UndefinedKeyword,
unique: SyntaxKind.UniqueKeyword,
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ export function transformDeclarations(context: TransformationContext): Transform
decl.importClause.isTypeOnly,
visibleDefaultBinding,
/*namedBindings*/ undefined,
decl.importClause.phase,
),
rewriteModuleSpecifier(decl, decl.moduleSpecifier),
tryGetResolutionModeOverride(decl.attributes),
Expand All @@ -905,6 +906,7 @@ export function transformDeclarations(context: TransformationContext): Transform
decl.importClause.isTypeOnly,
visibleDefaultBinding,
namedBindings,
decl.importClause.phase,
),
rewriteModuleSpecifier(decl, decl.moduleSpecifier),
tryGetResolutionModeOverride(decl.attributes),
Expand All @@ -921,6 +923,7 @@ export function transformDeclarations(context: TransformationContext): Transform
decl.importClause.isTypeOnly,
visibleDefaultBinding,
bindingList && bindingList.length ? factory.updateNamedImports(decl.importClause.namedBindings, bindingList) : undefined,
decl.importClause.phase,
),
rewriteModuleSpecifier(decl, decl.moduleSpecifier),
tryGetResolutionModeOverride(decl.attributes),
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/transformers/jsx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
getSemanticJsxChildren,
Identifier,
idText,
ImportPhase,
ImportSpecifier,
insertStatementAfterCustomPrologue,
isExpression,
Expand Down Expand Up @@ -172,7 +173,7 @@ export function transformJsx(context: TransformationContext): (x: SourceFile | B
for (const [importSource, importSpecifiersMap] of arrayFrom(currentFileState.utilizedImplicitRuntimeImports.entries())) {
if (isExternalModule(node)) {
// Add `import` statement
const importStatement = factory.createImportDeclaration(/*modifiers*/ undefined, factory.createImportClause(/*isTypeOnly*/ false, /*name*/ undefined, factory.createNamedImports(arrayFrom(importSpecifiersMap.values()))), factory.createStringLiteral(importSource), /*attributes*/ undefined);
const importStatement = factory.createImportDeclaration(/*modifiers*/ undefined, factory.createImportClause(/*isTypeOnly*/ false, /*name*/ undefined, factory.createNamedImports(arrayFrom(importSpecifiersMap.values())), ImportPhase.Evaluation), factory.createStringLiteral(importSource), /*attributes*/ undefined);
setParentRecursive(importStatement, /*incremental*/ false);
statements = insertStatementAfterCustomPrologue(statements.slice(), importStatement);
}
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/transformers/module/esnextAnd2015.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
idText,
ImportDeclaration,
ImportEqualsDeclaration,
ImportPhase,
insertStatementsAfterCustomPrologue,
isExportNamespaceAsDefaultDeclaration,
isExternalModule,
Expand Down Expand Up @@ -224,6 +225,7 @@ export function transformECMAScriptModule(context: TransformationContext): (x: S
factory.createNamedImports([
factory.createImportSpecifier(/*isTypeOnly*/ false, factory.createIdentifier("createRequire"), createRequireName),
]),
ImportPhase.Evaluation,
),
factory.createStringLiteral("module"),
/*attributes*/ undefined,
Expand Down Expand Up @@ -356,6 +358,7 @@ export function transformECMAScriptModule(context: TransformationContext): (x: S
factory.createNamespaceImport(
synthName,
),
ImportPhase.Evaluation,
),
updatedModuleSpecifier!,
node.attributes,
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/ts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2286,7 +2286,7 @@ export function transformTypeScript(context: TransformationContext): Transformer
// Elide the import clause if we elide both its name and its named bindings.
const name = shouldEmitAliasDeclaration(node) ? node.name : undefined;
const namedBindings = visitNode(node.namedBindings, visitNamedImportBindings, isNamedImportBindings);
return (name || namedBindings) ? factory.updateImportClause(node, /*isTypeOnly*/ false, name, namedBindings) : undefined;
return (name || namedBindings) ? factory.updateImportClause(node, /*isTypeOnly*/ false, name, namedBindings, node.phase) : undefined;
}

/**
Expand Down
Loading
Loading