From ef2decff7d6a1a0cb7a63384e1b2bda23e7c6aa1 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:42:50 -0700 Subject: [PATCH] create-next-app: fix font file corruption when using import alias (#69806) There's code that uses `glob` to find all template files and replaces the default import alias with whatever is specified during CNA. This does so without excluding fonts, and so we're unintentionally corrupting these `woff` files. This wasn't a problem in previous versions because we didn't use `localFont` in the default template, just `Inter`. The files were technically still being corrupted it just never manifested unless you went to actually use them. This is a quick fix to introduce minimal changes but ideally in a follow-up we should figure out a better solution for replacing aliases, ie by using an allowlist rather than an exclude list. h/t to @lubieowoce for the thorough test cases Fixes #69748 --------- Co-authored-by: Janka Uryga --- packages/create-next-app/templates/index.ts | 11 ++- .../create-next-app/templates/matrix.test.ts | 79 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 test/integration/create-next-app/templates/matrix.test.ts diff --git a/packages/create-next-app/templates/index.ts b/packages/create-next-app/templates/index.ts index ffbcd544c9b58..8c6e59a5e94ef 100644 --- a/packages/create-next-app/templates/index.ts +++ b/packages/create-next-app/templates/index.ts @@ -103,7 +103,16 @@ export const installTemplate = async ({ stats: false, // We don't want to modify compiler options in [ts/js]config.json // and none of the files in the .git folder - ignore: ["tsconfig.json", "jsconfig.json", ".git/**/*"], + // TODO: Refactor this to be an allowlist, rather than a denylist, + // to avoid corrupting files that weren't intended to be replaced + + ignore: [ + "tsconfig.json", + "jsconfig.json", + ".git/**/*", + "**/fonts/**", + "**/favicon.ico", + ], }); const writeSema = new Sema(8, { capacity: files.length }); await Promise.all( diff --git a/test/integration/create-next-app/templates/matrix.test.ts b/test/integration/create-next-app/templates/matrix.test.ts new file mode 100644 index 0000000000000..c5965debb21c1 --- /dev/null +++ b/test/integration/create-next-app/templates/matrix.test.ts @@ -0,0 +1,79 @@ +import { run, tryNextDev, useTempDir } from '../utils' + +describe.each(['app', 'pages'] as const)( + 'CNA options matrix - %s', + (pagesOrApp) => { + let nextTgzFilename: string + + beforeAll(() => { + if (!process.env.NEXT_TEST_PKG_PATHS) { + throw new Error('This test needs to be run with `node run-tests.js`.') + } + + const pkgPaths = new Map( + JSON.parse(process.env.NEXT_TEST_PKG_PATHS) + ) + + nextTgzFilename = pkgPaths.get('next')! + }) + + const isApp = pagesOrApp === 'app' + + const allFlagValues = { + app: [isApp ? '--app' : '--no-app'], + turbo: [process.env.TURBOPACK ? '--turbo' : '--no-turbo'], + + ts: ['--js', '--ts'], + importAlias: [ + '--import-alias=@acme/*', + '--import-alias=@/*', + '--no-import-alias', + ], + // doesn't affect if the app builds or not + // eslint: ['--eslint', '--no-eslint'], + eslint: ['--eslint'], + + srcDir: ['--src-dir', '--no-src-dir'], + tailwind: ['--tailwind', '--no-tailwind'], + + // shouldn't affect if the app builds or not + // packageManager: ['--use-npm', '--use-pnpm', '--use-yarn', '--use-bun'], + } + + const getPermutations = (items: T[][]): T[][] => { + if (!items.length) return [[]] + const [first, ...rest] = items + const children = getPermutations(rest) + return first.flatMap((value) => + children.map((child) => [value, ...child]) + ) + } + + const flagPermutations = getPermutations(Object.values(allFlagValues)) + const testCases = flagPermutations.map((flags) => ({ + name: flags.join(' '), + flags, + })) + + let id = 0 + it.each(testCases)('$name', async ({ flags }) => { + await useTempDir(async (cwd) => { + const projectName = `cna-matrix-${pagesOrApp}-${id++}` + const { exitCode } = await run( + [projectName, ...flags], + nextTgzFilename, + { + cwd, + } + ) + expect(exitCode).toBe(0) + + await tryNextDev({ + cwd, + projectName, + isApp, + }) + }) + }) + } +)