fix(vite): migration should handle config object correctly #20921 (#2…

…3364)     ## 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
nrwl · May 14, 2024 · 88297dd · 88297dd
1 parent 3e84967
commit 88297dd
Show file tree

Hide file tree

Showing 4 changed files with 153 additions and 7 deletions.
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
@@ -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`] = `
+"/// <reference types="vitest" />
+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 <base href="/app">\` 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('<head>', \`<head>\\n    <base href="\${BASE_HREF}">\`);
+    },
+  };
+};
+
+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
@@ -0,0 +1,64 @@
+export const viteConfigFixture = `/// <reference types="vitest" />
+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 <base href="/app">\` 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('<head>', \`<head>\\n    <base href="\${BASE_HREF}">\`);
+    },
+  };
+};
+
+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
@@ -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
@@ -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;