Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code splitting issue with 0.11.5 #1122

Closed
SamyPesse opened this issue Apr 7, 2021 · 4 comments
Closed

Code splitting issue with 0.11.5 #1122

SamyPesse opened this issue Apr 7, 2021 · 4 comments

Comments

@SamyPesse
Copy link
Contributor

Following #1088, 0.11.5 worked for our code-base, everything is loading correctly except dynamic import (code splitting)

Code like:

       case 'markup-templating':
            return import('prismjs/components/prism-markup-templating.min.js');
        case 'dns-zone-file':
            return import('prismjs/components/prism-dns-zone-file.min.js');
        case 't4-templating':
            return import('prismjs/components/prism-t4-templating.min.js');
        case 'markup':
            return import('prismjs/components/prism-markup.min.js');
        case 'docker':
            return import('prismjs/components/prism-docker.min.js');

is compiled to:

113725568-b8686200-96f3-11eb-9d7e-888444b31930

and fails during import with:

Uncaught (in promise) ReferenceError: require is not defined

the configuration looks like:

    const options: esbuild.BuildOptions = {
        entryPoints: {
            public: path.join(packagesDir, 'app-web/src/index.public.ts'),
            app: path.join(packagesDir, 'app-web/src/index.ts'),
        },
        metafile: true,
        outdir: webOutputDir,
        platform: 'browser',
        target: BROWSER_TARGET,
        splitting: true,
        format: 'esm',
        resolveExtensions: WEB_EXTS,
        external: [],
        loader: LOADERS,
        assetNames: 'assets/[name]-[hash]',
        chunkNames: 'chunks/[name]-[hash]',
        entryNames: isDEV ? '[name]' : '[name]-[hash]',
        ...esbuildOptions,
    };

I'm still investigating the root cause and I'll post updates with my findings.

@evanw
Copy link
Owner

evanw commented Apr 7, 2021

One thing to check is whether BROWSER_TARGET includes any browsers that don't support the import() syntax. When that's the case, esbuild avoids generating code containing import() under the assumption that import() is potentially a syntax error in those browsers. The browser versions which esbuild considers to support dynamic import are from https://caniuse.com/es6-module-dynamic-import and are as follows:

  • Chrome: 63+
  • Edge: 79+
  • Firefox: 67+
  • Mobile Safari: 11+
  • Desktop Safari: 11.1+

Of course require() won't work by default either. But at least it fails loudly instead of silently. The only other option I can think of here is to make doing this a build failure instead of having a successful build. Assuming this is the issue, what do you think about that?

@SamyPesse
Copy link
Contributor Author

Good catch 👍 thanks!

The value was:

const BROWSER_TARGET = ['chrome61', 'firefox60', 'safari11', 'edge18'];

It's now working with:

const BROWSER_TARGET = ['chrome63', 'firefox67', 'safari11.1', 'edge79'];

Of course require() won't work by default either. But at least it fails loudly instead of silently. The only other option I can think of here is to make doing this a build failure instead of having a successful build. Assuming this is the issue, what do you think about that?

It'd be easier to debug as a build failure 👍 Runtime errors are harder to notice, especially when code splitting is used to load "less important" code components.

A warning could also work: still generate import() call but notifies that it may not work at runtime. My thinking is that changing the browser targets to make code splitting work also impacts other syntaxes that could have worked on chrome61, etc.

@SamyPesse
Copy link
Contributor Author

@evanw otherwise why not always compile to import() but notify people to include a polyfill ?

@ericmatthys
Copy link

One thing to check is whether BROWSER_TARGET includes any browsers that don't support the import() syntax. When that's the case, esbuild avoids generating code containing import() under the assumption that import() is potentially a syntax error in those browsers.

I think this is problematic when using esbuild with other bundlers that look for the import syntax. If a Chrome browser target is less than Chrome 63 but webpack is handling imports and code splitting, esbuild is preventing that code splitting that was otherwise working in the old browser. Seeing as esbuild is being commonly used as a fast transform alongside other bundlers, it seems like this could be a use case worth considering, potentially as an opt-in. I think the only other option with webpack would be to use the legacy require.ensure function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants