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

ImportAttributes should go through the same emit phases when in an ImportTypeNode #56395

Merged
merged 3 commits into from Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions src/compiler/emitter.ts
Expand Up @@ -223,6 +223,7 @@ import {
isGeneratedIdentifier,
isGeneratedPrivateIdentifier,
isIdentifier,
isImportAttributes,
isIncrementalCompilation,
isInJsonFile,
isInternalDeclaration,
Expand Down Expand Up @@ -1840,6 +1841,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
if (hint === EmitHint.IdentifierName) return emitIdentifier(cast(node, isIdentifier));
if (hint === EmitHint.JsxAttributeValue) return emitLiteral(cast(node, isStringLiteral), /*jsxAttributeEscape*/ true);
if (hint === EmitHint.MappedTypeParameter) return emitMappedTypeParameter(cast(node, isTypeParameterDeclaration));
if (hint === EmitHint.ImportTypeNodeAttributes) return emitImportTypeNodeAttributes(cast(node, isImportAttributes));
if (hint === EmitHint.EmbeddedStatement) {
Debug.assertNode(node, isEmptyStatement);
return emitEmptyStatement(/*isEmbeddedStatement*/ true);
Expand Down Expand Up @@ -2944,15 +2946,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
if (node.attributes) {
writePunctuation(",");
writeSpace();
writePunctuation("{");
writeSpace();
writeKeyword(node.attributes.token === SyntaxKind.AssertKeyword ? "assert" : "with");
writePunctuation(":");
writeSpace();
const elements = node.attributes.elements;
emitList(node.attributes, elements, ListFormat.ImportAttributes);
writeSpace();
writePunctuation("}");
pipelineEmit(EmitHint.ImportTypeNodeAttributes, node.attributes);
}
writePunctuation(")");
if (node.qualifier) {
Expand Down Expand Up @@ -4077,6 +4071,18 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
writeTrailingSemicolon();
}

function emitImportTypeNodeAttributes(node: ImportAttributes) {
writePunctuation("{");
writeSpace();
writeKeyword(node.token === SyntaxKind.AssertKeyword ? "assert" : "with");
writePunctuation(":");
writeSpace();
const elements = node.elements;
emitList(node, elements, ListFormat.ImportAttributes);
writeSpace();
writePunctuation("}");
}

function emitImportAttributes(node: ImportAttributes) {
emitTokenWithComment(node.token, node.pos, writeKeyword, node);
writeSpace();
Expand Down
15 changes: 8 additions & 7 deletions src/compiler/types.ts
Expand Up @@ -8141,13 +8141,14 @@ export const enum ExternalEmitHelpers {

// dprint-ignore
export const enum EmitHint {
SourceFile, // Emitting a SourceFile
Expression, // Emitting an Expression
IdentifierName, // Emitting an IdentifierName
MappedTypeParameter, // Emitting a TypeParameterDeclaration inside of a MappedTypeNode
Unspecified, // Emitting an otherwise unspecified node
EmbeddedStatement, // Emitting an embedded statement
JsxAttributeValue, // Emitting a JSX attribute value
SourceFile, // Emitting a SourceFile
Expression, // Emitting an Expression
IdentifierName, // Emitting an IdentifierName
MappedTypeParameter, // Emitting a TypeParameterDeclaration inside of a MappedTypeNode
Unspecified, // Emitting an otherwise unspecified node
EmbeddedStatement, // Emitting an embedded statement
JsxAttributeValue, // Emitting a JSX attribute value
ImportTypeNodeAttributes,// Emitting attributes as part of an ImportTypeNode
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how much this matters given we seem to be adding stuff here, I do wonder if new things should be marked as internal. @rbuckton probably knows better than me for this bit.

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 it's fine for this to be public, as it is observable to custom transformers.

}

/** @internal */
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Expand Up @@ -7892,6 +7892,7 @@ declare namespace ts {
Unspecified = 4,
EmbeddedStatement = 5,
JsxAttributeValue = 6,
ImportTypeNodeAttributes = 7,
}
enum OuterExpressionKinds {
Parentheses = 1,
Expand Down
27 changes: 27 additions & 0 deletions tests/cases/fourslash/refactorToReturnTypeWithImportAssertions.ts
@@ -0,0 +1,27 @@
/// <reference path="fourslash.ts" />

// @module: NodeNext
// @traceResolution: true

// @filename: node_modules/inner/index.d.mts
//// export const esm = true;

// @filename: node_modules/inner/package.json
//// { "name": "inner", "exports": { "./mjs": "./index.mjs" } }

// @filename: foo.ts
//// export /*a*/function/*b*/ fn() {
//// return import("inner/mjs")
//// }

goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Infer function return type",
actionName: "Infer function return type",
actionDescription: "Infer function return type",
newContent:
`export function fn(): Promise<typeof import("/tests/cases/fourslash/node_modules/inner/index", { with: { "resolution-mode": "import" } })> {
return import("inner/mjs")
}`
});