diff --git a/changelog_unreleased/typescript/14688.md b/changelog_unreleased/typescript/14688.md new file mode 100644 index 000000000..7dd93092e --- /dev/null +++ b/changelog_unreleased/typescript/14688.md @@ -0,0 +1,15 @@ +#### Fix missing required comma in type parameters (#14688 by @fisker, @sosukesuzuki) + +Previously, we only print trailing comma when file extension is `.tsx`, turns out `.mts`, `.cts` files requires it to parse too. + + +```tsx +// Input +export const unsafeCoerce = (u: unknown): T => u as T + +// Prettier stable +export const unsafeCoerce = (u: unknown): T => u as T; + +// Prettier main +export const unsafeCoerce = (u: unknown): T => u as T; +``` diff --git a/scripts/build/config.js b/scripts/build/config.js index 1cba3ead7..688975506 100644 --- a/scripts/build/config.js +++ b/scripts/build/config.js @@ -185,7 +185,7 @@ const pluginFiles = [ process: (text) => text.replace( 'require("path")', - "{extname: file => file.split('.').pop()}" + '{extname: file => "." + file.split(".").pop()}' ), }, { diff --git a/src/language-js/parse/typescript.js b/src/language-js/parse/typescript.js index 3784071d4..d4c9349ef 100644 --- a/src/language-js/parse/typescript.js +++ b/src/language-js/parse/typescript.js @@ -7,7 +7,7 @@ import postprocess from "./postprocess/index.js"; import { throwErrorForInvalidNodes } from "./postprocess/typescript.js"; /** @type {import("@typescript-eslint/typescript-estree").TSESTreeOptions} */ -const parseOptions = { +const baseParseOptions = { // `jest@<=26.4.2` rely on `loc` // https://github.com/facebook/jest/issues/10444 // Set `loc` and `range` to `true` also prevent AST traverse @@ -15,7 +15,6 @@ const parseOptions = { loc: true, range: true, comment: true, - jsx: true, tokens: true, loggerFn: false, project: [], @@ -37,18 +36,34 @@ function createParseError(error) { }); } -function parse(text) { +// https://typescript-eslint.io/architecture/parser/#jsx +const isKnownFileType = (filepath) => + /\.(?:js|mjs|cjs|jsx|ts|mts|cts|tsx)$/i.test(filepath); + +function getParseOptionsCombinations(text, options) { + const filepath = options?.filepath; + if (filepath && isKnownFileType(filepath)) { + return [{ ...baseParseOptions, filePath: filepath }]; + } + + const shouldEnableJsx = isProbablyJsx(text); + return [ + { ...baseParseOptions, jsx: shouldEnableJsx }, + { ...baseParseOptions, jsx: !shouldEnableJsx }, + ]; +} + +function parse(text, options) { const textToParse = replaceHashbang(text); - const jsx = isProbablyJsx(text); + const parseOptionsCombinations = getParseOptionsCombinations(text, options); let result; try { - result = tryCombinations([ - // Try passing with our best guess first. - () => parseWithNodeMaps(textToParse, { ...parseOptions, jsx }), - // But if we get it wrong, try the opposite. - () => parseWithNodeMaps(textToParse, { ...parseOptions, jsx: !jsx }), - ]); + result = tryCombinations( + parseOptionsCombinations.map( + (parseOptions) => () => parseWithNodeMaps(textToParse, parseOptions) + ) + ); } catch ({ errors: [ // Suppose our guess is correct, throw the first error diff --git a/src/language-js/print/type-parameters.js b/src/language-js/print/type-parameters.js index c681f9a79..b0dcad8c2 100644 --- a/src/language-js/print/type-parameters.js +++ b/src/language-js/print/type-parameters.js @@ -12,7 +12,6 @@ import { isTestCall, hasComment, CommentCheckFlags, - isTSXFile, shouldPrintComma, getFunctionParameters, isObjectType, @@ -27,6 +26,20 @@ import { printTypeScriptMappedTypeModifier } from "./mapped-type.js"; const getTypeParametersGroupId = createGroupIdMapper("typeParameters"); +// Keep comma if the file extension not `.ts` and +// has one type parameter that isn't extend with any types. +// Because, otherwise formatted result will be invalid as tsx. +function shouldForceTrailingComma(path, options, paramsKey) { + const { node } = path; + return ( + getFunctionParameters(node).length === 1 && + node.type.startsWith("TS") && + !node[paramsKey][0].constraint && + path.parent.type === "ArrowFunctionExpression" && + !(options.filepath && /\.ts$/.test(options.filepath)) + ); +} + function printTypeParameters(path, options, print, paramsKey) { const { node } = path; @@ -68,16 +81,10 @@ function printTypeParameters(path, options, print, paramsKey) { ]; } - // Keep comma if the file extension is .tsx and - // has one type parameter that isn't extend with any types. - // Because, otherwise formatted result will be invalid as tsx. const trailingComma = node.type === "TSTypeParameterInstantiation" // https://github.com/microsoft/TypeScript/issues/21984 ? "" - : getFunctionParameters(node).length === 1 && - isTSXFile(options) && - !node[paramsKey][0].constraint && - path.parent.type === "ArrowFunctionExpression" + : shouldForceTrailingComma(path, options, paramsKey) ? "," : shouldPrintComma(options) ? ifBreak(",") diff --git a/src/language-js/utils/index.js b/src/language-js/utils/index.js index 4dc06fd12..a75d6f8ce 100644 --- a/src/language-js/utils/index.js +++ b/src/language-js/utils/index.js @@ -730,10 +730,6 @@ function identity(x) { return x; } -function isTSXFile(options) { - return options.filepath && /\.tsx$/i.test(options.filepath); -} - /** * @param {any} options * @param {("es5" | "all")} [level] @@ -1151,7 +1147,6 @@ export { isStringPropSafeToUnquote, isTemplateOnItsOwnLine, isTestCall, - isTSXFile, isTypeAnnotationAFunction, isNextLineEmpty, needsHardlineAfterDanglingComment, diff --git a/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap b/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap index 09287ad25..87806c395 100644 --- a/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap @@ -32,7 +32,7 @@ exports[`export-declare.ts [typescript] format 1`] = ` 2 |" `; -exports[`invalid-jsx-1.ts [typescript] format 1`] = ` +exports[`invalid-jsx-1.tsx [typescript] format 1`] = ` "Unexpected token. Did you mean \`{'>'}\` or \`>\`? (3:45) 1 | // https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-9.html#-and--are-now-invalid-jsx-text-characters 2 | diff --git a/tests/format/misc/errors/typescript/invalid-jsx-1.ts b/tests/format/misc/errors/typescript/invalid-jsx-1.tsx similarity index 100% rename from tests/format/misc/errors/typescript/invalid-jsx-1.ts rename to tests/format/misc/errors/typescript/invalid-jsx-1.tsx diff --git a/tests/format/typescript/comments/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/comments/__snapshots__/jsfmt.spec.js.snap index 855ae3735..2874d7786 100644 --- a/tests/format/typescript/comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/typescript/comments/__snapshots__/jsfmt.spec.js.snap @@ -62,7 +62,7 @@ abstract class AbstractFoo { ================================================================================ `; -exports[`after_jsx_generic.ts format 1`] = ` +exports[`after_jsx_generic.tsx format 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 80 @@ -293,7 +293,7 @@ export type AsyncExecuteOptions = child_process$execFileOpts & { ================================================================================ `; -exports[`jsx.ts format 1`] = ` +exports[`jsx.tsx format 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 80 diff --git a/tests/format/typescript/comments/after_jsx_generic.ts b/tests/format/typescript/comments/after_jsx_generic.tsx similarity index 100% rename from tests/format/typescript/comments/after_jsx_generic.ts rename to tests/format/typescript/comments/after_jsx_generic.tsx diff --git a/tests/format/typescript/comments/jsx.ts b/tests/format/typescript/comments/jsx.tsx similarity index 100% rename from tests/format/typescript/comments/jsx.ts rename to tests/format/typescript/comments/jsx.tsx diff --git a/tests/format/typescript/last-argument-expansion/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/last-argument-expansion/__snapshots__/jsfmt.spec.js.snap index f08c77347..3b7f037b8 100644 --- a/tests/format/typescript/last-argument-expansion/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/typescript/last-argument-expansion/__snapshots__/jsfmt.spec.js.snap @@ -38,7 +38,7 @@ export default class AddAssetHtmlPlugin { ================================================================================ `; -exports[`decorated-function.ts format 1`] = ` +exports[`decorated-function.tsx format 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 80 @@ -198,7 +198,7 @@ var listener = DOM.listen( ================================================================================ `; -exports[`forward-ref.ts format 1`] = ` +exports[`forward-ref.tsx format 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 80 diff --git a/tests/format/typescript/last-argument-expansion/decorated-function.ts b/tests/format/typescript/last-argument-expansion/decorated-function.tsx similarity index 100% rename from tests/format/typescript/last-argument-expansion/decorated-function.ts rename to tests/format/typescript/last-argument-expansion/decorated-function.tsx diff --git a/tests/format/typescript/last-argument-expansion/forward-ref.ts b/tests/format/typescript/last-argument-expansion/forward-ref.tsx similarity index 100% rename from tests/format/typescript/last-argument-expansion/forward-ref.ts rename to tests/format/typescript/last-argument-expansion/forward-ref.tsx diff --git a/tests/format/typescript/tsx/comma/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/tsx/comma/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..98a3127bf --- /dev/null +++ b/tests/format/typescript/tsx/comma/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,131 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`snippet: test.cjs format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.cts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.js format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.jsx format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.mjs format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.mts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.tsx format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: test.unknown format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; + +exports[`snippet: unnamed format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const A = () => {} +=====================================output===================================== +const A = () => {}; + +================================================================================ +`; diff --git a/tests/format/typescript/tsx/comma/jsfmt.spec.js b/tests/format/typescript/tsx/comma/jsfmt.spec.js new file mode 100644 index 000000000..8e6ac1416 --- /dev/null +++ b/tests/format/typescript/tsx/comma/jsfmt.spec.js @@ -0,0 +1,20 @@ +const code = "const A = () => {}"; + +run_spec( + { + importMeta: import.meta, + snippets: [ + "test.js", + "test.cjs", + "test.mjs", + "test.ts", + "test.jsx", + "test.mts", + "test.cts", + "test.tsx", + "test.unknown", + undefined, + ].map((filename) => ({ code, filename, name: filename ?? "unnamed" })), + }, + ["typescript"] +); diff --git a/tests/format/typescript/typeparams/print-width-120/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/typeparams/print-width-120/__snapshots__/jsfmt.spec.js.snap index 666c9bb03..f90f92763 100644 --- a/tests/format/typescript/typeparams/print-width-120/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/typescript/typeparams/print-width-120/__snapshots__/jsfmt.spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`issue-7542.ts - {"printWidth":120} format 1`] = ` +exports[`issue-7542.tsx - {"printWidth":120} format 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 120 diff --git a/tests/format/typescript/typeparams/print-width-120/issue-7542.ts b/tests/format/typescript/typeparams/print-width-120/issue-7542.tsx similarity index 100% rename from tests/format/typescript/typeparams/print-width-120/issue-7542.ts rename to tests/format/typescript/typeparams/print-width-120/issue-7542.tsx diff --git a/tests/integration/__tests__/format.js b/tests/integration/__tests__/format.js index 543d44d55..80342eb0b 100644 --- a/tests/integration/__tests__/format.js +++ b/tests/integration/__tests__/format.js @@ -24,7 +24,7 @@ test("typescript parser should throw the first error when both JSX and non-JSX m label: `; await expect( - prettier.format(input, { parser: "typescript" }) + prettier.format(input, { parser: "typescript", filepath: "foo.unknown" }) ).rejects.toThrowErrorMatchingSnapshot(); });