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

remove "require" and "import" warnings #1155

Merged
merged 2 commits into from
Apr 15, 2021
Merged

remove "require" and "import" warnings #1155

merged 2 commits into from
Apr 15, 2021

Conversation

evanw
Copy link
Owner

@evanw evanw commented Apr 15, 2021

Previously esbuild had warnings when bundling about uses of require and import that are not of the form require(<string literal>) or import(<string literal>). These warnings existed because the bundling process must be able to statically-analyze all dynamic imports to determine which files must be included. Here are some real-world examples of cases that esbuild doesn't statically analyze:

  • From mongoose:

    require('./driver').set(require(global.MONGOOSE_DRIVER_PATH));
  • From moment:

    aliasedRequire = require;
    aliasedRequire('./locale/' + name);
  • From logform:

    function exposeFormat(name) {
      Object.defineProperty(format, name, {
        get() { return require(`./${name}.js`); }
      });
    }
    exposeFormat('align');

All of these dynamic imports will not be bundled (i.e. they will be left as-is) and will crash at run-time if they are evaluated. Some of these crashes are ok since the code paths may have error handling or the code paths may never be used. Other crashes are not ok because the crash will actually be hit.

The warning from esbuild existed to let you know that esbuild is aware that it's generating a potentially broken bundle. If you discover that your bundle is broken, it's nice to have a warning from esbuild to point out where the problem is. And it was just a warning so the build process still finishes and successfully generates output files. If you didn't want to see the warning, it was easy to turn it off via --log-level=error.

However, there have been quite a few complaints about this warning. Some people seem to not understand the difference between a warning and an error, and think the build has failed even though output files were generated. Other people do not want to see the warning but also do not want to enable --log-level=error.

This PR removes this warning for both require and import. Now when you try to bundle code with esbuild that contains dynamic imports not of the form require(<string literal>) or import(<string literal>), esbuild will just silently generate a potentially broken bundle. This may affect people coming from other bundlers that support certain forms of dynamic imports that are not compatible with esbuild such as the Webpack-specific dynamic import() with pattern matching.

Related past issues:

@mattfysh
Copy link

Nice :) Wondering if instead of a build time warning, could the offending code be modified to log out a warning if the code path is entered at runtime?

@evanw
Copy link
Owner Author

evanw commented Apr 25, 2021

A note for anyone who ends up here:

You may be able to get more information to debug a crash due to unbundled require or import by using --log-level=debug. Logging of debug information for some of these cases was added in a later release. The difference is that the log level defaults to info so debug information is not logged by default.

@evanw
Copy link
Owner Author

evanw commented May 29, 2022

With the latest release, you can now use --log-override:unsupported-require-call=warning to make this a warning again (or --log-override:unsupported-require-call=error to make this an error).

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

Successfully merging this pull request may close these issues.

Checking for existence of require in if throws warning
2 participants