Merge pull request #7412 from maiieul/better-preview-chunk-names-in-debug-mode

refactor(rollup): move build mode + debug:true chunk names logic out of manualChunks

Author: wmertens

packages/qwik/src/optimizer/src/plugins/plugin.ts

@@ -891,14 +891,7 @@ export const manifest = ${JSON.stringify(manifest)};\n`;
     // Maybe a better solution would be to mark those files as entires earlier in the chain so that we can remove this check and the one above altogether.
     // We check .(tsx|jsx) after node_modules in case some node_modules end with .jsx or .tsx.
     if (/\.(tsx|jsx)$/.test(id)) {
-      const optimizer = getOptimizer();
-      const path = optimizer.sys.path;
-      const relativePath = path.relative(optimizer.sys.cwd(), id);
-      const sanitizedPath = relativePath
-        .replace(/^(\.\.\/)+/, '')
-        .replace(/^\/+/, '')
-        .replace(/\//g, '-');
-      return sanitizedPath; // We return sanitizedPath for qwikVite plugin with debug:true
+      return id;
     }
 
     return null;

packages/qwik/src/optimizer/src/plugins/rollup.ts

@@ -78,6 +78,7 @@ export function qwikRollup(qwikRollupOpts: QwikRollupPluginOptions = {}): any {
 
     outputOptions(rollupOutputOpts) {
       return normalizeRollupOutputOptionsObject(
+        qwikPlugin.getOptimizer(),
         qwikPlugin.getOptions(),
         rollupOutputOpts,
         false,
@@ -160,6 +161,7 @@ export function qwikRollup(qwikRollupOpts: QwikRollupPluginOptions = {}): any {
 }
 
 export function normalizeRollupOutputOptions(
+  optimizer: Optimizer,
   opts: NormalizedQwikPluginOptions,
   rollupOutputOpts: Rollup.OutputOptions | Rollup.OutputOptions[] | undefined,
   useAssetsDir: boolean,
@@ -173,18 +175,31 @@ export function normalizeRollupOutputOptions(
     }
 
     return rollupOutputOpts.map((outputOptsObj) => ({
-      ...normalizeRollupOutputOptionsObject(opts, outputOptsObj, useAssetsDir, manualChunks),
+      ...normalizeRollupOutputOptionsObject(
+        optimizer,
+        opts,
+        outputOptsObj,
+        useAssetsDir,
+        manualChunks
+      ),
       dir: outDir || outputOptsObj.dir,
     }));
   }
 
   return {
-    ...normalizeRollupOutputOptionsObject(opts, rollupOutputOpts, useAssetsDir, manualChunks),
+    ...normalizeRollupOutputOptionsObject(
+      optimizer,
+      opts,
+      rollupOutputOpts,
+      useAssetsDir,
+      manualChunks
+    ),
     dir: outDir || rollupOutputOpts?.dir,
   };
 }
 
 export function normalizeRollupOutputOptionsObject(
+  optimizer: Optimizer,
   opts: NormalizedQwikPluginOptions,
   rollupOutputOptsObj: Rollup.OutputOptions | undefined,
   useAssetsDir: boolean,
@@ -201,9 +216,31 @@ export function normalizeRollupOutputOptionsObject(
         ? `${opts.assetsDir}/${assetFileNames}`
         : assetFileNames;
     }
-    // Friendly name in dev or preview with debug mode
-    const fileName =
-      opts.buildMode == 'production' && !opts.debug ? 'build/q-[hash].js' : 'build/[name].js';
+
+    let fileName: string | ((chunkInfo: Rollup.PreRenderedChunk) => string) | undefined;
+    if (opts.buildMode === 'production' && !opts.debug) {
+      fileName = 'build/q-[hash].js';
+    } else {
+      // Friendlier names in dev or preview with debug mode
+      fileName = (chunkInfo) => {
+        if (chunkInfo.moduleIds.some((id) => id.endsWith('core.prod.mjs'))) {
+          return 'build/core.js';
+        }
+        if (chunkInfo.moduleIds.some((id) => id.endsWith('qwik-city/lib/index.qwik.mjs'))) {
+          return 'build/qwik-city.js';
+        }
+
+        // The chunk name can often be a path. We sanitize it to use dashes instead of slashes, to keep the same folder structure as without debug:true.
+        // Besides, Rollup doesn't accept absolute or relative paths as inputs for the [name] placeholder for the same reason.
+        const path = optimizer.sys.path;
+        const relativePath = path.relative(optimizer.sys.cwd(), chunkInfo.name);
+        const sanitized = relativePath
+          .replace(/^(\.\.\/)+/, '')
+          .replace(/^\/+/, '')
+          .replace(/\//g, '-');
+        return `build/${sanitized}.js`;
+      };
+    }
     // client production output
     if (!outputOpts.entryFileNames) {
       outputOpts.entryFileNames = useAssetsDir ? `${opts.assetsDir}/${fileName}` : fileName;

packages/qwik/src/optimizer/src/plugins/vite.ts

@@ -360,6 +360,7 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any {
         updatedViteConfig.build!.rollupOptions = {
           input: opts.input,
           output: normalizeRollupOutputOptions(
+            qwikPlugin.getOptimizer(),
             opts,
             viteConfig.build?.rollupOptions?.output,
             useAssetsDir,

packages/qwik/src/optimizer/src/plugins/vite.unit.ts

@@ -12,6 +12,29 @@ import {
 
 const cwd = process.cwd();
 
+const chunkInfoMocks: Rollup.PreRenderedChunk[] = [
+  {
+    exports: [''],
+    name: 'chunk.tsx',
+    facadeModuleId: 'chunk.tsx',
+    isDynamicEntry: false,
+    isEntry: false,
+    isImplicitEntry: false,
+    moduleIds: ['chunk.tsx'],
+    type: 'chunk',
+  },
+  {
+    exports: [''],
+    name: cwd + '/app/chunk.tsx',
+    facadeModuleId: cwd + '/app/chunk.tsx',
+    isDynamicEntry: false,
+    isEntry: false,
+    isImplicitEntry: false,
+    moduleIds: [cwd + '/app/chunk.tsx'],
+    type: 'chunk',
+  },
+];
+
 function mockOptimizerOptions(): OptimizerOptions {
   return {
     sys: {
@@ -61,6 +84,12 @@ test('command: serve, mode: development', async () => {
   const build = c.build!;
   const rollupOptions = build!.rollupOptions!;
   const outputOptions = rollupOptions.output as Rollup.OutputOptions;
+  const chunkFileNames = outputOptions.chunkFileNames as (
+    chunkInfo: Rollup.PreRenderedChunk
+  ) => string;
+  const entryFileNames = outputOptions.entryFileNames as (
+    chunkInfo: Rollup.PreRenderedChunk
+  ) => string;
 
   assert.deepEqual(opts.target, 'client');
   assert.deepEqual(opts.buildMode, 'development');
@@ -71,8 +100,10 @@ test('command: serve, mode: development', async () => {
   assert.deepEqual(rollupOptions.input, normalizePath(resolve(cwd, 'src', 'entry.dev')));
 
   assert.deepEqual(outputOptions.assetFileNames, 'assets/[hash]-[name].[ext]');
-  assert.deepEqual(outputOptions.chunkFileNames, 'build/[name].js');
-  assert.deepEqual(outputOptions.entryFileNames, 'build/[name].js');
+  assert.deepEqual(chunkFileNames(chunkInfoMocks[0]), `build/chunk.tsx.js`);
+  assert.deepEqual(entryFileNames(chunkInfoMocks[0]), `build/chunk.tsx.js`);
+  assert.deepEqual(chunkFileNames(chunkInfoMocks[1]), 'build/app-chunk.tsx.js');
+  assert.deepEqual(entryFileNames(chunkInfoMocks[1]), 'build/app-chunk.tsx.js');
   assert.deepEqual(outputOptions.format, 'es');
 
   assert.deepEqual(build.dynamicImportVarsOptions?.exclude, [/./]);
@@ -131,6 +162,12 @@ test('command: build, mode: development', async () => {
   const build = c.build!;
   const rollupOptions = build!.rollupOptions!;
   const outputOptions = rollupOptions.output as Rollup.OutputOptions;
+  const chunkFileNames = outputOptions.chunkFileNames as (
+    chunkInfo: Rollup.PreRenderedChunk
+  ) => string;
+  const entryFileNames = outputOptions.entryFileNames as (
+    chunkInfo: Rollup.PreRenderedChunk
+  ) => string;
 
   assert.deepEqual(opts.target, 'client');
   assert.deepEqual(opts.buildMode, 'development');
@@ -144,8 +181,10 @@ test('command: build, mode: development', async () => {
   assert.deepEqual(rollupOptions.input, [normalizePath(resolve(cwd, 'src', 'root'))]);
 
   assert.deepEqual(outputOptions.assetFileNames, 'assets/[hash]-[name].[ext]');
-  assert.deepEqual(outputOptions.chunkFileNames, 'build/[name].js');
-  assert.deepEqual(outputOptions.entryFileNames, 'build/[name].js');
+  assert.deepEqual(chunkFileNames(chunkInfoMocks[0]), `build/chunk.tsx.js`);
+  assert.deepEqual(entryFileNames(chunkInfoMocks[0]), `build/chunk.tsx.js`);
+  assert.deepEqual(chunkFileNames(chunkInfoMocks[1]), 'build/app-chunk.tsx.js');
+  assert.deepEqual(entryFileNames(chunkInfoMocks[1]), 'build/app-chunk.tsx.js');
 
   assert.deepEqual(build.dynamicImportVarsOptions?.exclude, [/./]);
   assert.deepEqual(build.ssr, undefined);