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

[improvement] split external modules in dynamic imports #2854

Closed
its-dibo opened this issue Jan 20, 2023 · 6 comments
Closed

[improvement] split external modules in dynamic imports #2854

its-dibo opened this issue Jan 20, 2023 · 6 comments

Comments

@its-dibo
Copy link

when you add './module' into the externals array and then importing it dynamically (or even statically) like this import('./module')
it compiled into Promise.resolve() instead of a real dynamic import.

as per the docs, this is an intended behavior, so this is not an issue report.
but this has some drawbacks, especially if the imported module includes a singleton reference.

what's wrong with this behavior?

  • if the imported module is a singleton, then not splitting it causes this singleton module is included separately in each importer, which causes unexpected results
  • most developers expected that because the module was added to the excluded and it was built separately, so the importer imported it from the compiled file, but no, it loads a different version of it, which is the bundled one.
  • it is difficult to figure out why the singleton function returns undefined, even after reviewing the code carefully and the setup of esbuild and the externals array

as a dirty work arround we can do import(`${'./module'}`), but it also has drowbacks.

  • it loses its types, typescript will not able to extract the type of this file from the source version of it unless the developer is intended to import from a file than is not exists in the source code and will be generated in compile-time
  • as mentioned before, many developers may forget to convert a simple string into a nonsense variable and expect a good result.

as a rule of thumb, if the imported module matches an external flag, split the code even if it is analyzable.

@hyrious
Copy link

hyrious commented Jan 20, 2023

Wait, did you mean import('./module') in your source code is bundled into Promise.resolve().then(() => require('./module'))? This shouldn't happen unless you've set the target environment to not support dynamic import (like node12, chrome59, etc.).

If you're expecting code splitting, please use the real splitting option (--format=esm --splitting), although it only works in esm format.

@its-dibo
Copy link
Author

its-dibo commented Jan 20, 2023

Promise.resolve().then(() => require('./module'))

no, the problem is not it compiled into the commonjs require instead of import,
it compiled into something similar to:
var { setApp: setApp2 } = await Promise.resolve().then(() => (init_config(), config_exports));
which means that ./module is bundled in the compiled result, it is not imported nor required externally

I target Esnext and use format:esm
I set ,splitting to false for two reasons:

  • not all modules I want to split, only modules that marked as externals
  • I find the same file compiled multiple times with different hashes (the dist folder is removed before the building), maybe it occurs because the same file is imported from multiple files or any other reason

I want all external modules to be splitted regardless of the splitting flag.
@hyrious

@evanw
Copy link
Owner

evanw commented Jan 22, 2023

I'm not totally sure what you're saying. I don't think you actually mean "external". Setting something as "external" means "don't bundle this" but it seems like you actually do want these to be bundled, just put in separate chunks?

After reading what you wrote, my best guess as to what you're asking is that you want to enable esbuild's code splitting but have more control over code splitting chunk boundaries. So then this would be a duplicate of #207 perhaps.

@its-dibo
Copy link
Author

its-dibo commented Feb 6, 2023

but it seems like you actually do want these to be bundled, just put in separate chunks

no, I meen don't bundle it at all

@evanw
Copy link
Owner

evanw commented Feb 9, 2023

Please provide a self-contained way to reproduce the issue.

@evanw
Copy link
Owner

evanw commented Apr 7, 2023

I’m closing this issue because a way to reproduce it was not provided.

@evanw evanw closed this as not planned Won't fix, can't repro, duplicate, stale Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants