From 893ccf5512d9ea5cd7b130644ae8718e584cd7ea Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 11 Jul 2024 22:29:59 +0900 Subject: [PATCH] Do not overwrite file deps during project initialization (#2318) * Do not overwrite file deps during project initialization * Debug test * Improve error debugging * Bring back styling unit tests * Debug flaky tests * Restore tests to make it fail again * Fix issue with transpiling node_modules on CI * Restore the other transpilation test * Restore other tests --- packages/cli/src/commands/hydrogen/setup.ts | 12 ++-- packages/cli/src/lib/onboarding/common.ts | 15 +++-- packages/cli/src/lib/onboarding/local.test.ts | 65 +++++++++++++++++++ packages/cli/src/lib/onboarding/local.ts | 7 +- .../cli/src/lib/setups/routes/generate.ts | 24 ++++--- packages/cli/src/lib/transpile/project.ts | 2 + 6 files changed, 106 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/commands/hydrogen/setup.ts b/packages/cli/src/commands/hydrogen/setup.ts index 164dab09f3..6b94f68cc4 100644 --- a/packages/cli/src/commands/hydrogen/setup.ts +++ b/packages/cli/src/commands/hydrogen/setup.ts @@ -115,11 +115,13 @@ export async function runSetup(options: RunSetupOptions) { ]), ) .then(async () => { - routes = await setupRoutes( - rootDirectory, - typescript ? 'ts' : 'js', - i18n, - ); + routes = await setupRoutes(rootDirectory, typescript ? 'ts' : 'js', { + i18nStrategy: i18n, + // User might have added files before running this command. + // We should overwrite them to ensure the routes are set up correctly. + // Relies on Git to restore the files if needed. + overwriteFileDeps: true, + }); }); } diff --git a/packages/cli/src/lib/onboarding/common.ts b/packages/cli/src/lib/onboarding/common.ts index 8e8ced1b0a..ca6343c4fc 100644 --- a/packages/cli/src/lib/onboarding/common.ts +++ b/packages/cli/src/lib/onboarding/common.ts @@ -139,7 +139,7 @@ export async function handleRouteGeneration( setupRoutes: async ( directory: string, language: Language, - i18nStrategy?: I18nStrategy, + options?: {i18nStrategy?: I18nStrategy; overwriteFileDeps?: boolean}, ) => { if (needsRouteGeneration) { const result = await generateRoutes( @@ -148,8 +148,10 @@ export async function handleRouteGeneration( directory, force: true, typescript: language === 'ts', - localePrefix: i18nStrategy === 'subfolders' ? 'locale' : false, + localePrefix: + options?.i18nStrategy === 'subfolders' ? 'locale' : false, signal: controller.signal, + ...options, }, { rootDirectory: directory, @@ -760,8 +762,13 @@ export function createAbortHandler( ), ); - // Enable this when debugging tests: - // if (process.env.NODE_ENV === 'test') console.error(error); + if (process.env.SHOPIFY_UNIT_TEST && process.exit.name !== 'spy') { + // This is not an artificial error for testing, print it and + // throw an unhandled rejection. Otherwise, the error will be + // swallowed by the test runner after process.exit is called. + console.error('Error during test before process.exit:', error); + throw error; + } // This code runs asynchronously so throwing here // turns into an unhandled rejection. Exit process instead: diff --git a/packages/cli/src/lib/onboarding/local.test.ts b/packages/cli/src/lib/onboarding/local.test.ts index 76abae04e9..4aed0fcdd2 100644 --- a/packages/cli/src/lib/onboarding/local.test.ts +++ b/packages/cli/src/lib/onboarding/local.test.ts @@ -177,6 +177,71 @@ describe('local templates', () => { }); }); + describe('styling libraries', () => { + it('scaffolds Tailwind CSS', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await setupTemplate({ + path: tmpDir, + git: false, + language: 'ts', + styling: 'tailwind', + routes: true, + }); + + // Injects dependencies + const packageJson = await readFile(`${tmpDir}/package.json`); + expect(packageJson).toMatch(/"@tailwindcss\/vite": "/); + + // Copies Tailwind file + await expect( + readFile(`${tmpDir}/app/styles/tailwind.css`), + ).resolves.toMatch(/@import 'tailwindcss';/); + + // Injects styles in Root + const rootFile = await readFile(`${tmpDir}/app/root.tsx`); + await expect(rootFile).toMatch(/import tailwindCss from/); + await expect(rootFile).toMatch( + /export function links\(\) \{.*?return \[.*\{rel: 'stylesheet', href: tailwindCss\}/ims, + ); + + // Adds the Vite plugin + const viteConfig = await readFile(`${tmpDir}/vite.config.ts`); + await expect(viteConfig).toMatch(/tailwindcss\(\)/); + + const output = outputMock.info(); + expect(output).toMatch('success'); + expect(output).not.toMatch('warning'); + expect(output).toMatch(/Styling:\s*Tailwind/); + }); + }); + + it('scaffolds Vanilla Extract', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await setupTemplate({ + path: tmpDir, + git: false, + language: 'ts', + styling: 'vanilla-extract', + routes: true, + }); + + // Injects dependencies + const packageJson = await readFile(`${tmpDir}/package.json`); + expect(packageJson).toMatch(/"@vanilla-extract\/vite-plugin": "/); + expect(packageJson).toMatch(/"@vanilla-extract\/css": "/); + + // Adds the Vite plugin + const viteConfig = await readFile(`${tmpDir}/vite.config.ts`); + await expect(viteConfig).toMatch(/vanillaExtractPlugin\(\)/); + + const output = outputMock.info(); + expect(output).toMatch('success'); + expect(output).not.toMatch('warning'); + expect(output).toMatch(/Styling:\s*Vanilla Extract/); + }); + }); + }); + describe('i18n strategies', () => { it('scaffolds i18n with domains strategy', async () => { await inTemporaryDirectory(async (tmpDir) => { diff --git a/packages/cli/src/lib/onboarding/local.ts b/packages/cli/src/lib/onboarding/local.ts index a4268bb0fa..5ad9d7d8d4 100644 --- a/packages/cli/src/lib/onboarding/local.ts +++ b/packages/cli/src/lib/onboarding/local.ts @@ -335,7 +335,12 @@ export async function setupLocalStarterTemplate( setupSummary.i18nError = error as AbortError; }); - await setupRoutes(project.directory, language, i18nStrategy) + await setupRoutes(project.directory, language, { + i18nStrategy, + // The init process might have added and modified files. Do not overwrite them. + // E.g. CSS imports might have been added to the root. + overwriteFileDeps: false, + }) .then((routes) => { setupSummary.routes = routes; diff --git a/packages/cli/src/lib/setups/routes/generate.ts b/packages/cli/src/lib/setups/routes/generate.ts index 4712ef12c9..710319ca7f 100644 --- a/packages/cli/src/lib/setups/routes/generate.ts +++ b/packages/cli/src/lib/setups/routes/generate.ts @@ -175,6 +175,7 @@ type GenerateProjectFileOptions = { templatesRoot?: string; localePrefix?: string; signal?: AbortSignal; + overwriteFileDeps?: boolean; }; /** @@ -221,6 +222,7 @@ export async function generateProjectFile( localePrefix, v1RouteConvention = false, signal, + overwriteFileDeps = true, }: GenerateProjectFileOptions & { rootDirectory: string; appDirectory: string; @@ -233,15 +235,6 @@ export async function generateProjectFile( const extension = (routeFrom.match(/(\.[jt]sx?)$/) ?? [])[1] ?? '.tsx'; routeFrom = routeFrom.replace(extension, ''); - const routeTemplatePath = await getTemplateAppFile( - routeFrom + extension, - templatesRoot, - ); - const allFilesToGenerate = await findRouteDependencies( - routeTemplatePath, - await getTemplateAppFile('', templatesRoot), - ); - const routeDestinationPath = joinPath( appDirectory, getDestinationRoute(routeFrom, localePrefix, {v1RouteConvention}) + @@ -268,6 +261,15 @@ export async function generateProjectFile( result.operation = 'replaced'; } + const routeTemplatePath = await getTemplateAppFile( + routeFrom + extension, + templatesRoot, + ); + const allFilesToGenerate = await findRouteDependencies( + routeTemplatePath, + await getTemplateAppFile('', templatesRoot), + ); + for (const filePath of allFilesToGenerate) { const isRoute = filePath.startsWith(ASSETS_STARTER_DIR_ROUTES + '/'); const destinationPath = isRoute @@ -277,6 +279,10 @@ export async function generateProjectFile( filePath.replace(/\.ts(x?)$/, `.${typescript ? 'ts$1' : 'js$1'}`), ); + if (!overwriteFileDeps && (await fileExists(destinationPath))) { + continue; + } + // Create the directory if it doesn't exist. if (!(await fileExists(dirname(destinationPath)))) { await mkdir(dirname(destinationPath)); diff --git a/packages/cli/src/lib/transpile/project.ts b/packages/cli/src/lib/transpile/project.ts index 87b6951638..b77926810b 100644 --- a/packages/cli/src/lib/transpile/project.ts +++ b/packages/cli/src/lib/transpile/project.ts @@ -63,6 +63,8 @@ export async function transpileProject(projectDir: string, keepTypes = true) { const entries = await glob('**/*.+(ts|tsx)', { absolute: true, cwd: projectDir, + dot: true, + ignore: ['**/node_modules/**'], }); const formatConfig = await getCodeFormatOptions();