-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Support named exports in cjs libraries for optimizer #720
Comments
interesting note: |
Have you tried to add it to the include list of optimizeDeps? |
Anyone can give a whole repo for this? |
@underfin yep, I create canonical example from docs of react-dynamic-modules, but using vite instead webpack. Vite can't build it correctly. |
@Akiyamka please use following code and add
|
When running
I fear this is the only way to make it work during development. This is also how you would consume CommonJS modules in Node, by the way.
Webpack does not suffer from this because it is not transpiling CJS to ESM as Rollup is. Named exports are just treated as properties on the My big recommendation would be to create an issue for redux-dynamic-modules to provide a proper ESM build, though, or adapt their documentation to mention that their package cannot be used with named exports in a browser, and that by providing only a CommonJS build but promoting the use of named exports, they are relying on non-standardized custom behaviour in some bundlers instead of open web standards. |
So the only actionable point I see for now to solve this in a general way would be for vite to rewrite named imports to property accesses on the default import, e.g. // before
import {foo} from 'bar';
// output if 'bar' is originally CommonJS
import _$viteDefault from 'bar'; // just something that is unlikely to conflict with anything, otherwise you need to do deeper scope analysis
const {foo} = _$viteDefault; We could support this from |
Thank you for clarifying what is happening. You are right about the fact that is to apply to the maintainers in order that they have fixed the behavior to the appropriate specifications.
|
@lukastaegert Thanks for your support this,I'm willing work for it.
That is also |
That is my fear as well, considering React itself has been guilty of this pattern for a long time.
Putting this behind a flag seems sensible as another drawback would be that it would prevent live-bindings between modules. Which is likely not an issue for many, but still. So there would still be some pressure for libraries to adopt native ES modules. |
Yeah.Thanks for you suggestion.
Hope hear this progress, thanks. |
Out of curiosity, could someone point me to the code where Vite is transpiling CommonJS in the dev server? Is it just using |
Here.I it run the optimizer, it will bundle cjs before dev server start. vite/src/node/optimizer/index.ts Lines 182 to 193 in e24133e
It include |
So the dev server is not translating dependencies on-demand? Because I thought these were the issues we talked about. Or is this only done for CommonJS/dependencies only? But maybe I should actually set up a project to get a feeling for this to see how we can best help you here and how the issues above actually come to pass. |
Yeah, but that only support es module.The common js module must be transform to es module at first and then can use it inside
No.The optimizer is also can optimize es6 module which contains many sub-package imported. The repo https:/Akiyamka/vite-fail-build-repro is here. |
Maybe we can just scan the content whether inculde
|
If you just scan if the content contains these as sub-strings, there is a big risk of false positives which will cause more issues. Now what
So what we could do is give you access to this table from the plugin in some way. It will not be 100% straightforward, in your case you would likely have another plugin "talk" to the commonjs plugin (via a yet-to-be-created API, but really not too complicated) to get this table at the end of the build. Now what this will give you is a mapping of resolved module ids, i.e. full path names, to a boolean if Rollup thinks these files are CommonJS. Would this be enough for you? |
@lukastaegert.Yeah, that are sounds great. |
I'm not sure if related here, I'm using babylon.js with its loaders. import * as BABYLON from 'babylonjs';
import 'babylonjs-loaders'; But in dev, the import {c as createCommonjsModule, b as babylon, a as commonjsGlobal} from '/@modules/common/babylon-ae6b791e.js';
...
module.exports = t(babylon.__moduleExports); where I'd like to have this code just works, and is there any workaround now? |
This should be fixed by #837 |
Describe the bug
Fresh Vite app with react template.
From
Vite generate:
But this code crush with
Chrome
Firefox
How it exported
https:/microsoft/redux-dynamic-modules/blob/8d9bbbbb037abeae2b3f4f62345668097c0f8015/packages/redux-dynamic-modules-core/src/ModuleStore.ts#L99
At least with webpack it's work fine
Reproduction
I can create repo with this single import if required
System Info
vite
version: v1.0.0-rc.4Logs
Debug logs
The text was updated successfully, but these errors were encountered: