From 56b134dbf24a5d9352118641fe3eecd0bf4c8ee6 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Tue, 14 May 2024 14:21:22 +0100 Subject: [PATCH] fix(vite): migration should handle config object correctly #20921 (#23364) ## Current Behavior The migration to update vite config is incorrectly matching other object literal and arrow functions ## Expected Behavior Ensure more accurate updating of vite config file ## Related Issue(s) Fixes #20921 (cherry picked from commit 88297dd7273f7843d211b816a80039d99f59e26c) --- .../update-vite-config.spec.ts.snap | 69 +++++++++++++++++++ .../vite-config-with-additional-js.fixture.ts | 64 +++++++++++++++++ .../update-17-2-0/update-vite-config.spec.ts | 11 +++ .../update-17-2-0/update-vite-config.ts | 16 +++-- 4 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 packages/vite/src/migrations/update-17-2-0/lib/vite-config-with-additional-js.fixture.ts diff --git a/packages/vite/src/migrations/update-17-2-0/__snapshots__/update-vite-config.spec.ts.snap b/packages/vite/src/migrations/update-17-2-0/__snapshots__/update-vite-config.spec.ts.snap index 95c9c90531171..0493e2cfcbae1 100644 --- a/packages/vite/src/migrations/update-17-2-0/__snapshots__/update-vite-config.spec.ts.snap +++ b/packages/vite/src/migrations/update-17-2-0/__snapshots__/update-vite-config.spec.ts.snap @@ -270,6 +270,75 @@ export default defineConfig(({ mode }) => { " `; +exports[`change-vite-ts-paths-plugin migration should correctly migrate the vite config within the file and not other object literals 1`] = ` +"/// +import react from '@vitejs/plugin-react'; +import dns from 'dns'; +import { PluginOption, defineConfig } from 'vite'; +import { nxViteTsPaths } from '@nx/vite/plugins/nx-tsconfig-paths.plugin'; + +dns.setDefaultResultOrder('verbatim'); +const BASE_HREF = '/app'; + +/** + * Adds \` to the head of the index.html + * The reason why the \`base\` configuration property doesn't work is because it makes + * all assets served under \`/app\` of \`/\` and this impacts the download zip service worker + * We only want to the service worker to listen to events related to downloads, but not capture any other events + * and the only way to do this is make sure all assets are served from the root, but we still want our app path to be \`/app\` + * + * This mimics the same behavior we had with webpack before migrating to vite + */ +const baseHrefPlugin: () => PluginOption = () => { + return { + name: 'html-transform', + transformIndexHtml(html) { + return html.replace('', \`\\n \`); + }, + }; +}; + +export default defineConfig({ + root: __dirname, + cacheDir: '../../node_modules/.vite/jetstream', + envPrefix: 'NX', + + server: { + port: 4200, + host: 'localhost', + }, + + build: { + outDir: '../../dist/apps/demo', + reportCompressedSize: true, + commonjsOptions: { transformMixedEsModules: true }, // Put all assets at the root of the app instead of under /assets + assetsDir: './', + sourcemap: true, + rollupOptions: { + output: { + sourcemap: true, + }, + }, + }, + + plugins: [ + react({ + jsxImportSource: '@emotion/react', + babel: { + plugins: ['@emotion/babel-plugin'], + }, + }), + nxViteTsPaths(), + baseHrefPlugin(), + ], + + worker: { + plugins: [nxViteTsPaths()], + }, +}); +" +`; + exports[`change-vite-ts-paths-plugin migration should show warning to the user if could not recognize config 1`] = ` "// some invalid config " diff --git a/packages/vite/src/migrations/update-17-2-0/lib/vite-config-with-additional-js.fixture.ts b/packages/vite/src/migrations/update-17-2-0/lib/vite-config-with-additional-js.fixture.ts new file mode 100644 index 0000000000000..3105b0b97870a --- /dev/null +++ b/packages/vite/src/migrations/update-17-2-0/lib/vite-config-with-additional-js.fixture.ts @@ -0,0 +1,64 @@ +export const viteConfigFixture = `/// +import react from '@vitejs/plugin-react'; +import dns from 'dns'; +import { PluginOption, defineConfig } from 'vite'; +import { nxViteTsPaths } from '@nx/vite/plugins/nx-tsconfig-paths.plugin'; + +dns.setDefaultResultOrder('verbatim'); +const BASE_HREF = '/app'; + +/** + * Adds \` to the head of the index.html + * The reason why the \`base\` configuration property doesn't work is because it makes + * all assets served under \`/app\` of \`/\` and this impacts the download zip service worker + * We only want to the service worker to listen to events related to downloads, but not capture any other events + * and the only way to do this is make sure all assets are served from the root, but we still want our app path to be \`/app\` + * + * This mimics the same behavior we had with webpack before migrating to vite + */ +const baseHrefPlugin: () => PluginOption = () => { + return { + name: 'html-transform', + transformIndexHtml(html) { + return html.replace('', \`\\n \`); + }, + }; +}; + +export default defineConfig({ + cacheDir: '../../node_modules/.vite/jetstream', + envPrefix: 'NX', + + server: { + port: 4200, + host: 'localhost', + }, + + build: { + // Put all assets at the root of the app instead of under /assets + assetsDir: './', + sourcemap: true, + rollupOptions: { + output: { + sourcemap: true, + }, + }, + }, + + plugins: [ + react({ + jsxImportSource: '@emotion/react', + babel: { + plugins: ['@emotion/babel-plugin'], + }, + }), + nxViteTsPaths(), + baseHrefPlugin(), + ], + + worker: { + plugins: [ + nxViteTsPaths(), + ], + }, +});`; diff --git a/packages/vite/src/migrations/update-17-2-0/update-vite-config.spec.ts b/packages/vite/src/migrations/update-17-2-0/update-vite-config.spec.ts index d30297a49db98..9cf7725700c4e 100644 --- a/packages/vite/src/migrations/update-17-2-0/update-vite-config.spec.ts +++ b/packages/vite/src/migrations/update-17-2-0/update-vite-config.spec.ts @@ -7,6 +7,7 @@ import { } from '@nx/devkit'; import updateBuildDir from './update-vite-config'; +import { viteConfigFixture } from './lib/vite-config-with-additional-js.fixture'; describe('change-vite-ts-paths-plugin migration', () => { let tree: Tree; @@ -71,6 +72,16 @@ describe('change-vite-ts-paths-plugin migration', () => { ) ); }); + + it('should correctly migrate the vite config within the file and not other object literals', async () => { + // ARRANGE + addProject1(tree, 'demo'); + tree.write(`apps/demo/vite.config.ts`, viteConfigFixture); + // ACT + await updateBuildDir(tree); + // ASSERT + expect(tree.read('apps/demo/vite.config.ts', 'utf-8')).toMatchSnapshot(); + }); }); function addProject1(tree: Tree, name: string) { diff --git a/packages/vite/src/migrations/update-17-2-0/update-vite-config.ts b/packages/vite/src/migrations/update-17-2-0/update-vite-config.ts index 13ffa53de0733..e2ba23607d6ab 100644 --- a/packages/vite/src/migrations/update-17-2-0/update-vite-config.ts +++ b/packages/vite/src/migrations/update-17-2-0/update-vite-config.ts @@ -61,16 +61,18 @@ export function getConfigNode(configFileContents: string): ts.Node | undefined { } let configNode = tsquery.query( configFileContents, - `ObjectLiteralExpression` + `CallExpression:has(Identifier[name=defineConfig]) > ObjectLiteralExpression` )?.[0]; - const arrowFunctionReturnStatement = tsquery.query( - configFileContents, - `ArrowFunction Block ReturnStatement ObjectLiteralExpression` - )?.[0]; + if (!configNode) { + const arrowFunctionReturnStatement = tsquery.query( + configFileContents, + `ArrowFunction Block ReturnStatement ObjectLiteralExpression` + )?.[0]; - if (arrowFunctionReturnStatement) { - configNode = arrowFunctionReturnStatement; + if (arrowFunctionReturnStatement) { + configNode = arrowFunctionReturnStatement; + } } return configNode;