Skip to content

Commit

Permalink
Do not overwrite file deps during project initialization (#2318)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
frandiox authored Jul 11, 2024
1 parent 76e9285 commit 893ccf5
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 19 deletions.
12 changes: 7 additions & 5 deletions packages/cli/src/commands/hydrogen/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
});
}

Expand Down
15 changes: 11 additions & 4 deletions packages/cli/src/lib/onboarding/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
65 changes: 65 additions & 0 deletions packages/cli/src/lib/onboarding/local.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/lib/onboarding/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
24 changes: 15 additions & 9 deletions packages/cli/src/lib/setups/routes/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ type GenerateProjectFileOptions = {
templatesRoot?: string;
localePrefix?: string;
signal?: AbortSignal;
overwriteFileDeps?: boolean;
};

/**
Expand Down Expand Up @@ -221,6 +222,7 @@ export async function generateProjectFile(
localePrefix,
v1RouteConvention = false,
signal,
overwriteFileDeps = true,
}: GenerateProjectFileOptions & {
rootDirectory: string;
appDirectory: string;
Expand All @@ -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}) +
Expand All @@ -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
Expand All @@ -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));
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/lib/transpile/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 893ccf5

Please sign in to comment.