web-infra-dev / rsbuild

The Rspack-based build tool. It's fast, out-of-the-box and extensible.
https://rsbuild.dev/
MIT License
1.75k stars 137 forks source link

[Bug]: webpackChunkName magic comments sometimes not respected #3570

Closed matteom-synth closed 1 month ago

matteom-synth commented 1 month ago

Version

System:
    OS: macOS 14.6.1
    CPU: (11) arm64 Apple M3 Pro
    Memory: 4.08 GB / 36.00 GB
    Shell: 3.7.1 - /opt/homebrew/bin/fish
  Browsers:
    Chrome: 129.0.6668.60
    Safari: 17.6
  npmPackages:
    @rsbuild/core: 1.0.7 => 1.0.7
    @rsbuild/plugin-react: 1.0.2 => 1.0.2
    @rsbuild/plugin-svgr: 1.0.2 => 1.0.2
    @rsbuild/plugin-type-check: 1.0.1 => 1.0.1

Details

Hello again! I hate opening issues without reproducible examples but I'm a bit lost here.
I believe something changed between 1.0.1-rc.3 and 1.0.5 and now (when using the default split by experience) the webpackChunkName is not respected anymore for us in some cases.
This is breaking the rsbuild plugin shared here: https://mmazzarolo.com/blog/2024-08-13-async-chunk-preloading-on-load/.

It just seem that in some cases async chunks are generated correctly but don't use the name as set in webpackChunkName anymore: the name used in webpackChunkName don't show up in compilation.namedChunkGroups nor in the stats file.

I can't pinpoint why, though, but I can reproduce the issue consistently switching between 1.0.1-rc.3 and 1.0.5 (in our big codebase). I can also still reproduce it on 1.07.

This week I was also pinged by a couple of folks wrt to the rsbuild plugin article posted above telling me that it doesn't work. I thought it was some issues on their end, but now that I had a chance to update our app, I guess it might be related to this issue.

Does this ring any bell? Any recent change to the way the magic comments or code splitting is happening? Thanks!

(If it can help, in our rsbuild we're not setting any cutom performance settings)

Reproduce link

No link :(

Reproduce Steps

I can't create a small reproducible example—sorry. I can reproduce it in my big SPA where, in a file with a ton of lazy loaded pages (some even prefetched), only one of them is actually being named correctly. I don't know why only that specific one. In 1.0.1-rc.3 they're all built and named correctly.

const Page1 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page1" */ `./pages/Page1`));
const Page2 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page2" */ `./pages/Page2`));
const Page3 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page3" */ "./pages/Page3"));
const Page4 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page4" */ `./pages/Page4`));
const Page5 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page5" */ `./pages/Page5`));
const Page6 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page6" */ `./pages/Page6`));
const Page7 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page7" */ `./pages/Page7`));
const Page8 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page8" */ `./pages/Page8`));

const Page9 = lazy(() => import(/* webpackChunkName: "Page9" */ `./pages/Page9`));
const Page10 = lazy(() => import(/* webpackChunkName: "Page10" */ `./pages/Page10`));
const Page11 = lazy(() => import(/* webpackChunkName: "Page11" */ `./pages/Page11`));
const Page12 = lazy(() => import(/* webpackChunkName: "Page12" */ `./pages/Page12`));
const Page13 = lazy(() => import(/* webpackChunkName: "Page13" */ `./pages/Page13`));
const Page14 = lazy(() => import(/* webpackChunkName: "Page14" */ `./pages/Page14`));
const Page15 = lazy(() => import(/* webpackChunkName: "Page15" */ `./pages/Page15`));
const Page16 = lazy(() => import(/* webpackChunkName: "Page16" */ `./pages/Page16`));
const Page17 = lazy(() => import(/* webpackChunkName: "Page17" */ `./pages/Page17`));
const Page18 = lazy(() => import(/* webpackChunkName: "Page18" */ `./pages/Page18`));
const Page19 = lazy(() => import(/* webpackChunkName: "Page19" */ `./pages/Page19`));
const Page20 = lazy(() => import(/* webpackChunkName: "Page20" */ `./pages/Page20`));
const Page21 = lazy(() => import(/* webpackChunkName: "Page21" */ `./pages/Page21`));
const Page22 = lazy(() => import(/* webpackChunkName: "Page22" */ `./pages/Page22`));
const Page23 = lazy(() => import(/* webpackChunkName: "Page23" */ `./pages/Page23`));
const Page24 = lazy(() => import(/* webpackChunkName: "Page24" */ `./pages/Page24`));
const Page25 = lazy(() => import(/* webpackChunkName: "Page25" */ `./pages/Page25`));
const Page26 = lazy(() => import(/* webpackChunkName: "Page26" */ `./pages/Page26`));
const Page27 = lazy(() => import(/* webpackChunkName: "Page27" */ `./pages/Page27`));
const Page28 = lazy(() => import(/* webpackChunkName: "Page28" */ `./pages/Page28`));
const Page29 = lazy(() => import(/* webpackChunkName: "Page29" */ `./pages/Page29`));
const Page30 = lazy(() => import(/* webpackChunkName: "Page30" */ `./pages/Page30`));
const Page31 = lazy(() => import(/* webpackChunkName: "Page31" */ `./pages/Page31`));
const Page32 = lazy(() => import(/* webpackChunkName: "Page32" */ `./pages/Page32`));
const Page33 = lazy(() => import(/* webpackChunkName: "Page33" */ `./pages/Page33`));
const Page34 = lazy(() => import(/* webpackChunkName: "Page34" */ `./pages/Page34`));
const Page35 = lazy(() => import(/* webpackChunkName: "Page35" */ `./pages/Page35`));

If I had to guess, maybe something changed in the way the default performance strategy work and now things are not being "named" unless >= a certaing threshold?

chenjiahan commented 1 month ago

Should related to https://github.com/web-infra-dev/rsbuild/pull/3510

@Timeless0911 cc

matteom-synth commented 1 month ago

@chenjiahan , @Timeless0911 , yup, can confirm that manually setting preserveAllComments: true in the config fixes it!

Curious as to why if it's set to false only some of the webpackChunkName are ignored, I would have expected that all of them will be ignore 🤷

Thanks for the quick response (as always!). This unblock us, but I assume you might want to deal with it in some ways on the rsbuild side too to avoid this to happen by default, right? Up to you to keep this issue open/close.

Timeless0911 commented 1 month ago

Can you try some of your code directly on swc playgroud? https://play.swc.rs/?version=1.7.28&code=H4sIAAAAAAAAA0utKMgvKlEoqSxIVQjOz00NATFsFaq5FBT0tbSApIKWgkNKakFRanJiSWoKWEAfSKbl59tbgZXBFWJRClWsoJCUWGSlUFxSlJmXbg0UqLXmAiIufX2FYqCdXMn5ecUlICOBNmtoKtjaKVSDpFMhboPIRgDl9LWU4%2BMDQoNc4%2BO19BWUIpSsuQBKgQ%2FcwAAAAA%3D%3D&config=H4sIAAAAAAAAAz2OSwrDMAxE9zmF0DqQ4mV3oScxQSku%2FiGppSb47lFC2p3mzRvQNgBgCjmsDe%2Bw%2Big0Hugli%2BXNToBpAqxMQvyhOcZHSYmyivXK71O3QfVswn9jRFpW%2FzWC2irJwqEqnmW%2FNur5SXoYJO7mHF48liL0e8dQH%2FoOZFuJfKgAAAA%3D

matteom-synth commented 1 month ago

Sure, what should I do specifically? As soon as I paste my code (or even just a simple single import with webpackChunkName) and enable one of mangle, minify, or compress, webpack magic comments are stripped away. Not sure if that's what you expect (I can't see an option for preserveAllComments in the playground)

matteom-synth commented 1 month ago

Ok, nevermind, I saw that the option is in the JSON editor side. If preserveAllComments is enabled, magic comments are not stripped (regardless of mangle, etc.). If it's set to fale or not set, they get stripped away.

Timeless0911 commented 1 month ago

Can you share me a playground of which magic comments are stripped away?

chenjiahan commented 1 month ago

I can not reproduce this issue:

image

Can you shared your rsbuild.config.ts or a minimal example?

matteom-synth commented 1 month ago

Can you share me a playground of which magic comments are stripped away?

Sure, here's one with no preserveAllComments and minify enabled.

Can you shared your rsbuild.config.ts or a minimal example?

Yes. I tried reducing the config to the bare minimum required to build our app. Can't get it slimmer than this, otherwise our app doesn't build, but I'd assume many of these settings are "safe" (such as output, html, server, etc.—I don't see them having any impact here)? Here it is:

import { pluginReact } from "@rsbuild/plugin-react";
import { pluginSvgr } from "@rsbuild/plugin-svgr";

import { webpackAliases } from "../config/aliases";

import { chunksPreloadingPlugin } from "./rsbuild-chunks-preloading-plugin";
import { monorepoPackagesToCompile, parseEnv } from "./utils";

import type { RsbuildConfig } from "@rsbuild/core";

const APP_PORT = 3000;

export const env = parseEnv();

const config: RsbuildConfig = {
  source: {
    entry: {
      index: "./src/index",
    },
    define: env.stringifiedEnvVars,
    alias: {
      ...webpackAliases,
    },
    include: [
      {
        and: monorepoPackagesToCompile.map((packagePath) => [
          packagePath,
          { not: /[\\/]node_modules[\\/]/ },
        ]),
      },
    ],
  },
  output: {
    distPath: {
      root: "build",
    },
  },
  html: {
    template: "./public/index.html",
    templateParameters: env.envVars,
  },
  server: {
    port: APP_PORT,
    host: "0.0.0.0",
    // We'll copy the public directory content manually, no need to do it
    // automatically here.
    publicDir: {
      copyOnBuild: false,
    },
  },
  dev: {
    assetPrefix: "/",
  },
  tools: {
    swc: {
      exclude: [".*\\.test\\.ts$", ".*\\.test\\.tsx$"],
      jsc: {
        parser: {
          syntax: "typescript",
          decorators: true,
        },
        transform: {
          /**
           * https://swc.rs/docs/configuration/compilation#jsctransformdecoratorversion
           * Legacy decorators for Motion Canvas components
           */
          decoratorVersion: "2021-12",
        },
        // preserveAllComments: true, // 👈👈👈👈 
        experimental: {
          plugins: [
            [
              "@swc/plugin-emotion",
              {
                // On local builds, we want to see the class names in the DOM
                // For some reasons, "dev-only" doesn't seem to work so we manually
                // toggle it here.
                // https://www.npmjs.com/package/@swc/plugin-emotion
                autoLabel:
                  process.env.NODE_ENV === "development" ? "always" : undefined,
              },
            ],
          ],
        },
      },
    },
  },
  plugins: [
    pluginReact({
      swcReactOptions: {
        importSource: "@emotion/react",
        runtime: "automatic",
      },
    }),
    pluginSvgr({
      mixedImport: true,
      svgrOptions: { prettier: false, svgo: false, titleProp: true, ref: true },
    }),
    chunksPreloadingPlugin(),
  ],
};

export default config;

With this, async chunk names aren't respected. But if we enable preserveAllComments (which from my understanding was enabled by default before), they do.

FWIW The main bulk of async chunks are defined within a single file for us (anonymized the file/components name):

// ...some other imports

/**
 * Here we start lazy loading some routes of the app.
 * In this section we have the most frequently accessed pages, which we decorate
 * with "webpackPrefetch" in order to prefetch them for better perceived performance.
 * Prefetched routes will be loaded in the background while the browser is idle,
 * ensuring they are immediately available upon user navigation.
 */
const Page1 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page1" */ `./pages/Page1`));
const Page2 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page2" */ `./pages/Page2`));
const Page3 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page3" */ "./pages/Page3"));
const Page4 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page4" */ `./pages/Page4`));
const Page5 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page5" */ `./pages/Page5`));
const Page6 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page6" */ `./pages/Page6`));
const Page7 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page7" */ `./pages/Page7`));
const Page8 = lazy(() => import(/* webpackPrefetch: true, webpackChunkName: "Page8" */ `./pages/Page8`));

/**
 * And this section lists all the remaining routes of the app (still lazy loaded).
 * If you're adding a new page, you should probably include it here :)
 */
const Page9 = lazy(() => import(/* webpackChunkName: "Page9" */ `./pages/Page9`));
const Page10 = lazy(() => import(/* webpackChunkName: "Page10" */ `./pages/Page10`));
const Page11 = lazy(() => import(/* webpackChunkName: "Page11" */ `./pages/Page11`));
const Page12 = lazy(() => import(/* webpackChunkName: "Page12" */ `./pages/Page12`));
const Page13 = lazy(() => import(/* webpackChunkName: "Page13" */ `./pages/Page13`));
const Page14 = lazy(() => import(/* webpackChunkName: "Page14" */ `./pages/Page14`));
const Page15 = lazy(() => import(/* webpackChunkName: "Page15" */ `./pages/Page15`));
const Page16 = lazy(() => import(/* webpackChunkName: "Page16" */ `./pages/Page16`));
const Page17 = lazy(() => import(/* webpackChunkName: "Page17" */ `./pages/Page17`));
const Page18 = lazy(() => import(/* webpackChunkName: "Page18" */ `./pages/Page18`));
const Page19 = lazy(() => import(/* webpackChunkName: "Page19" */ `./pages/Page19`));
const Page20 = lazy(() => import(/* webpackChunkName: "Page20" */ `./pages/Page20`));
const Page21 = lazy(() => import(/* webpackChunkName: "Page21" */ `./pages/Page21`));
const Page22 = lazy(() => import(/* webpackChunkName: "Page22" */ `./pages/Page22`));
const Page23 = lazy(() => import(/* webpackChunkName: "Page23" */ `./pages/Page23`));
const Page24 = lazy(() => import(/* webpackChunkName: "Page24" */ `./pages/Page24`));
const Page25 = lazy(() => import(/* webpackChunkName: "Page25" */ `./pages/Page25`));
const Page26 = lazy(() => import(/* webpackChunkName: "Page26" */ `./pages/Page26`));
const Page27 = lazy(() => import(/* webpackChunkName: "Page27" */ `./pages/Page27`));
const Page28 = lazy(() => import(/* webpackChunkName: "Page28" */ `./pages/Page28`));
const Page29 = lazy(() => import(/* webpackChunkName: "Page29" */ `./pages/Page29`));
const Page30 = lazy(() => import(/* webpackChunkName: "Page30" */ `./pages/Page30`));
const Page31 = lazy(() => import(/* webpackChunkName: "Page31" */ `./pages/Page31`));
const Page32 = lazy(() => import(/* webpackChunkName: "Page32" */ `./pages/Page32`));
const Page33 = lazy(() => import(/* webpackChunkName: "Page33" */ `./pages/Page33`));
const Page34 = lazy(() => import(/* webpackChunkName: "Page34" */ `./pages/Page34`));
const Page35 = lazy(() => import(/* webpackChunkName: "Page35" */ `./pages/Page35`));

// ...code
matteom-synth commented 1 month ago

OK! Found the issue. It's the double quotes vs "`" in the import statement. That's also why only a single one is imported (see the third one).

Can't find a way to reproduce it in the SWC playground, though...

matteom-synth commented 1 month ago

I tried creating a minimal reproducible project, but... I can't. Not sure why, but only in my huge ass prod project this is happening. If you don't have any idea on what could be happening here, that's OK, I guess we can close it?

chenjiahan commented 1 month ago

Yes, we've tried that as well, but we still can't reproduce the issue.

We can close this issue first, and if someone else encounters a similar problem and can provide a way to reproduce it, we will reopen it.