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 warning about using dynamic imports #453

Closed
vovacodes opened this issue Oct 13, 2020 · 5 comments
Closed

Remove warning about using dynamic imports #453

vovacodes opened this issue Oct 13, 2020 · 5 comments

Comments

@vovacodes
Copy link

Hey @evanw, thanks for your excellent job building esbuild, I'd like to discuss one thing:

As mentioned in #56:

as of version 0.5.3, dynamic imports with template literals with variables are now a warning, not an error. The dynamic import is passed through to the output code unchanged.

I don't think it should be a warning though, as using dynamic import() can be a totally valid use case when you need to load an ES module in the browser and you don't want the bundler to touch it at all.
I believe, the bundlers should not discourage users from using the standards. I do understand that esbuild needs to know the user's intent, which might indeed be code splitting, so there should be a way to clearly communicate that intent, and it's probably fine to use string templates vs string literals, but once I communicated it I don't expect to see any warnings about it, WDYT?

@nettybun
Copy link

Personally I prefer the warning because in a large code base with many dynamic imports it's a good reminder that some of my code might not be as well compiled/codesplit. Then I can adjust the import() to use a ternary or branch so it can have a string instead of a template. This also helps searching the codebase.

@evanw
Copy link
Owner

evanw commented Oct 14, 2020

I'm not sure what to think about this. I see both sides. I agree that the warning is annoying, but it's also annoying to have the build silently succeed only for the resulting bundle to be broken without an explanation. A similar warning is also triggered for dynamic require, although it is suppressed if the code is surrounded by a try/catch.

For example, many packages expect to be bundled with Webpack's file system emulation where they do expect the bundler to attempt to bundle dynamic imports. This warning will flag these cases.

One possible argument could be that import() returns a promise and code should in theory be handling the rejected promise. But I've seen many uses of import() that don't check for errors, in which case it would be hard to notice that something is wrong if the build was silent.

One solution that works today is to just call the API and filter the warnings yourself. I agree that's not great.

@vovacodes
Copy link
Author

Closing it, if someone has something to add, please reopen/create a new one.

@evanw
Copy link
Owner

evanw commented May 29, 2022

With the latest release, you can now configure the log level for this warning like this:

--log-override:unsupported-dynamic-import=silent
--log-override:unsupported-dynamic-import=warning
--log-override:unsupported-dynamic-import=error

@btakita
Copy link

btakita commented Jan 16, 2024

I have an isomorphic library which uses dynamic imports of node: modules to avoid esbuild errors for browser environments. These imports are only run on server environments, not run on browsers. I would prefer to have a comment to disable the warning for that particular case of dynamic import. Better yet, a comment to tell esbuild to treat the import as an external & ignore bundling that particular import, so I don't need to create a variable to get around the esbuild error.

This pattern of building isomorphic code is legitimate, as it is the "least bad" solution. So it should not cause a warning IMO.

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

4 participants