You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[@web/dev-server]: @rollup/plugin-commonjs version >18 breaks commonjs imports #1700
In the main branch, trying to run wds (npm run start) shows the following error when importing apexcharts or moment (those are the ones I tested):
Uncaught SyntaxError: The requested module '/__web-dev-server__/rollup/moment.js?web-dev-server-rollup-null-byte=%00%2FUsers%2Fpsiska%2Ftemp%2Fwds-commonjs-test%2Fnode_modules%2Fmoment%2Fmoment.js%3Fcommonjs-module' does not provide an export named 'exports'
I also have a simple rollup build in the repo, using the same plugins as in wds, and that one works (npm run build).
changed the title [-][@web/dev-server]: `@rollup/plugin-commonjs` version >18 breaks commonjs imports[/-][+][@web/dev-server]: `@rollup/plugin-commonjs` version `>18` breaks commonjs imports[/+]on Sep 28, 2021
The problem still exists. Installing the @rollup/plugin-commonjs@22.0.0-4 (beta) did not help but produces another error: Error while transforming node_modules/axios/index.js: Could not resolve import "commonjsHelpers.js".
I seem to have hit this as well. I was trying to run tests on code that uses jStat. Before I added @rollup/plugin-commonjs I was getting the error:
🚧 Browser logs:
TypeError: Cannot set properties of undefined (setting 'jStat')
at ..\..\node_modules\jstat\dist\jstat.js:7:22
at ..\..\node_modules\jstat\dist\jstat.js:9:3
Fair enough, since jStat is only provided in UMD format, I added the CommonJS plugin to my Web Test Runner config:
However, this led to me getting the following error when trying to run tests:
🚧 Browser logs:
SyntaxError: Octal escape sequences are not allowed in strict mode.
For the life of me, I couldn't figure out what was using octal escape sequences. I finally stumbled on this issue, and after pinning @rollup/plugin-commonjs to 18.1.0 everything now works fine. Note that I tried using 19.0.0 and the error was still there.
I'm posting this here in case it helps others realize that what seems like a completely different issue may in fact have this same cause.
EDIT:
Wanted to add that I have been using rollup with @rollup/plugin-commonjs at 21.0.2 in my actual build, and it pulls in jstat without any problems.
I have the same problem with @rollup/plugin-commonjs@22.0.0. I'm getting this error message:
Error while transforming node_modules/@dynatrace/openkit-js/dist/node/index.js: Could not resolve import "commonjsHelpers.js".
> 1 | import * as commonjsHelpers from "commonjsHelpers.js";
Using @rollup/plugin-commonjs@18.0.0 the errors becomes:
Error while transforming node_modules/axios/package.json: Unexpected "export"
Unfortunately, I haven't found a proper solution fo the problem.
What I did to bypass/evade the problem is using rollup-plugin-url-resolve to pull the package I need as a module (or transformed into a module) from unpkg and include it into the build.
Due to privacy/security concerns, loading it from a (public) cdn (in production) wasn't an option.
In my source file:
import { PACKAGE } from 'https://unpkg.com/@PACKAGE/package.min.js?module=1';
In my rollup.config.js:
import urlResolve from 'rollup-plugin-url-resolve';
...
plugins: [
urlResolve(),
...
I know it isn't a 'proper' fix and might not solve everybody's problems, but hope that it helps some of you.
Error while transforming node_modules/slugify/slugify.js: Could not resolve import "commonjsHelpers.js".
> 1 | import * as commonjsHelpers from "commonjsHelpers.js";
| ^
2 | import { __module as slugifyModule, exports as slugify } from "\u0000/home/me/git/projectname/node_modules/slugify/slugify.js?commonjs-module"
3 |
4 | (function (module, exports) {
Unfortunately, downgrading @rollup/plugin-commonjs to ^21 or lower causes an infinite loop somewhere in the code and the server eventually dies with out of memory.
but this won't work out of the box because of this issue (some nonsensical import in plugin-commonjs it seems).
similarly, maybe its worth you having docs on using commonjs in particular in WTR/dev-server? unless im just missing it, i can't seem to find a page explaining it. i had to figure it out from someone else's repo.
edit:
actually downgrading doesn't solve anything, i get the same inf loop balusc gets
edit2:
it seems the latest one falls over because this flow happens:
commonjs plugin injects some special import import x from 'commonjsHelpers.js';
WTR begins processing that import (to resolve it or w/e else)
node-resolve plugin starts, decides this import is nonsense and throws an error (a nice error, not uncaught)
commonjs plugin never runs for this path
in the commonjs plugin, they tell rollup to return/inject some fabricated module anytime something tries to import their special helpers name:
I managed to monkey-patch the commonjs module to now properly resolve the commonjsHelpers.js file. The Issue is that newer versions of the commonjs-plugin inject an auxiliary plugin at the start of rollups plugin list (see here).
Modifying the plugin-list during the options-hook and returning it is not supported by the rollup-adapter as it does not handle the return value here.
So everything works now? Sadly no. We are back where we started with the error message being does not provide an export named 'exports'. 🤷
Trying to minimally reproduce the issue I had above, I accidentally managed to reproduce the infinite loop problem first.
The commonjs-plugins resolveId calls rollups internal resolve function with the option skipSelf which according to rollup documentation prevents infinite loops. This flag seems to not be handled properly here, as it gets ignored if the plugin provides a custom resolveImport hook. I don't understand the motive for adding the exception, so I am unsure how to handle it.
Edit: I now also realize that the issue seems to involve the node-resolve functionality, as the infinite loops remains when filtering out the commonjs plugin from the resolve function
Edit: Looking at the rollup implementation, I figured that each plugin that calls resolve with the skipSelf flag needs to be saved into a set and excluded from loop. Without doing this, node-import and commonjs play a very advanced game of ping-pong.
I now found a solution for the third issue, regarding does not provide an export named 'exports'. I am not 100% sure but it appears to be an issue in the commonjs plugin itself, as the module stub it imports here does not actually contain the expected export here. Since it works just fine in rollup, I am not sure how to proceed with this.
All in all I created a set of monkey-patches you can load into the web-dev-server config and released them here. This should fix all three issues at play, please give it a try and provide some feedback, so I know that my assumptions are correct.
I hope to soon create a PR for this project and start a discussion on how to fix the remaining issue in the commonjs plugin.
this doesn't load because the node-resolve module executes first in WTR/dev-server and decides the import is invalid (unresolvable) before we even reach the commonjs plugin code. i guess thats why they prepend the plugin you mentioned, to reach it first.
seems we would have to fix plugin-commonjs since it is producing nonsensical code (the non-existent export).
Yes, it's three issues and they are all interconnected. Afaict the non-existing export is the only one within the commonjs plugin itself, it just doesn't seem to surface when bundling with rollup. I've reported that one here.
All three Issues have a workaround using the monkey-patch.
this doesn't load because the node-resolve module executes first
I don't think that this is true, because then my patch would not work. The commonjsHelpers.js import is also prefixed with an invisible null-byte, telling node-resolve to skip resolving it and handing it over to the commonjs plugin.
I don't think that this is true, because then my patch would not work. The commonjsHelpers.js import is also prefixed with an invisible null-byte, telling node-resolve to skip resolving it and handing it over to the commonjs plugin.
Fix the rollupadapter here to assign the return value - you can already see the ?? rollupInputOptions is meaningless (since it isn't a call to anything and isn't assigned to anything), so my guess is the original author meant to assign it but didn't
Fix @rollup/plugin-commonjs on their end since the import/export don't match up and clearly should (unless somehow we're not looking at the right import and/or export, i.e. the right pair)
Also fix in the rollupAdapter the fact it doesn't respect skipSelf - weirdly this does look like it should already work, would be good to know for sure why it doesn't
I just cloned @43081j's fork and switched to his PR branch (from #2050) and copied the built version of dev-server-rollup it into my node_modules folder and voila, it works!
@akrawitz thanks for putting me on to this! I see it was marked "Ready for Review" just an hour ago, excited for it to be included in an upcoming version!
For those of you interested, we're doing some testing against a canary release in this area via:
🦋 Creating git tags...
🦋 New tag: @web/dev-server@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-core@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-esbuild@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-hmr@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-import-maps@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-legacy@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-rollup@0.0.0-canary-20230216020153
🦋 New tag: @web/dev-server-storybook@0.0.0-canary-20230216020153
🦋 New tag: @web/test-runner-core@0.0.0-canary-20230216020153
@Westbrook I see @web/test-runner-core@0.0.0-canary-20230216020153 but not @web/test-runner - won't this mean that @web/test-runner will pick up the "latest" release of @web/test-runner-core because it's version constraint of ^0.10.27 doesn't match the canary version? I think you need a canary of @web/test-runner to pick up the canary of core, or people will need to add their npm/yarn resolutions.
I am testing with an import of the legacy tweetnacl library in my project.
With @rollup/plugin-commonjs@^18.0.0 it attempts to load the following URL, fails due to a 404, and the page load request hangs without spiking the CPU.
@pospi If those actually are dependents, then you're likely not leveraging the API for the Rollup Plugin as expected. Take a look at https://github.com/modernweb-dev/web/pull/2180/files#diff-71859c8cb2f330cca436fcf08f2826397cae98029c3b795a23b4b05703d56e06R207 where our test suite outlines this. The requireReturnsDefault: 'preferred', appears support the idea that your ESM code prefers to work with its imports as if they were ESM files, but the CJS that it imports will prefer to work with its imports as if they were CJS files. I'm not a specialist of this plugin, so you'll benefit from working with the Rollup community to confirm, but this appears to be the expected usage in this context.
Tried it as suggested ("rollup": "^3.15.0" and requireReturnsDefault: 'preferred') but unfortunately no change to the first failing URL output above.
Do I manually need to resolve this module myself? Why is this string present literally nowhere on Github let alone here or the Rollup repos and why are the Tutanota team giving it special handling? 😂
The particular problem that this thread refers to is one that did not happen with @rollup/plugin-commonjs@^18.0.0 - it only became an issue with more recent versions. So, if you are seeing your problem even with @rollup/plugin-commonjs@^18.0.0, then I suspect that the underlying cause of your issue may be different.
Does that mean there's a different existing issue I should be filing these logs under, or should I submit a new one?
Currently still encountering the same failure as per ^24.0.1above. Dependencies are:
npm run start will produce the error for multiple packages, output in the terminal.
Note: When using isomorphic-ws instead of ws in main.mjs, the error can be resolved by changing the file node_modules/isomorphic-ws/node.js from commonjs notation
Activity
[-][@web/dev-server]: `@rollup/plugin-commonjs` version >18 breaks commonjs imports[/-][+][@web/dev-server]: `@rollup/plugin-commonjs` version `>18` breaks commonjs imports[/+]Pratik29121 commentedon Sep 29, 2021
const basename = process.env.NODE_ENV === 'production' ? '/my-react-app' : '/';
return (
);
phihu commentedon Jan 11, 2022
The problem still exists. Installing the @rollup/plugin-commonjs@22.0.0-4 (beta) did not help but produces another error:
Error while transforming node_modules/axios/index.js: Could not resolve import "commonjsHelpers.js".akrawitz commentedon Mar 4, 2022
I seem to have hit this as well. I was trying to run tests on code that uses jStat. Before I added
@rollup/plugin-commonjsI was getting the error:Fair enough, since jStat is only provided in UMD format, I added the CommonJS plugin to my Web Test Runner config:
However, this led to me getting the following error when trying to run tests:
For the life of me, I couldn't figure out what was using octal escape sequences. I finally stumbled on this issue, and after pinning
@rollup/plugin-commonjsto18.1.0everything now works fine. Note that I tried using19.0.0and the error was still there.I'm posting this here in case it helps others realize that what seems like a completely different issue may in fact have this same cause.
EDIT:
Wanted to add that I have been using
rollupwith@rollup/plugin-commonjsat21.0.2in my actual build, and it pulls injstatwithout any problems.felipefreitas commentedon Apr 27, 2022
I have the same problem with
@rollup/plugin-commonjs@22.0.0. I'm getting this error message:Using
@rollup/plugin-commonjs@18.0.0the errors becomes:web-test-runner.config::plugins:
phihu commentedon Apr 27, 2022
Unfortunately, I haven't found a proper solution fo the problem.
What I did to bypass/evade the problem is using rollup-plugin-url-resolve to pull the package I need as a module (or transformed into a module) from unpkg and include it into the build.
Due to privacy/security concerns, loading it from a (public) cdn (in production) wasn't an option.
In my source file:
import { PACKAGE } from 'https://unpkg.com/@PACKAGE/package.min.js?module=1';In my rollup.config.js:
I know it isn't a 'proper' fix and might not solve everybody's problems, but hope that it helps some of you.
mihaisavezi commentedon Apr 27, 2022
Ended up bundling this particular library dependency and handling it separately. Here's how this looks.
The dependency in question is a dependency of my dependency and this works.
lib/entry.js
BalusC commentedon May 25, 2022
I'm facing the same problem when using https://www.npmjs.com/package/slugify 1.6.5 in my ts project with
Opening the webpage spits this error in console:
Unfortunately, downgrading
@rollup/plugin-commonjsto^21or lower causes an infinite loop somewhere in the code and the server eventually dies with out of memory.43081j commentedon Sep 30, 2022
@daKmoR any idea what to do here? i understand downgrading solves the problem, but this isn't obvious from any of your docs, examples, etc.
currently someone (like i did) might think "ok ill go install dev-server-rollup and plugin-commonjs, then job done".
e.g.
but this won't work out of the box because of this issue (some nonsensical import in plugin-commonjs it seems).
similarly, maybe its worth you having docs on using commonjs in particular in WTR/dev-server? unless im just missing it, i can't seem to find a page explaining it. i had to figure it out from someone else's repo.
edit:
actually downgrading doesn't solve anything, i get the same inf loop balusc gets
edit2:
it seems the latest one falls over because this flow happens:
import x from 'commonjsHelpers.js';in the commonjs plugin, they tell rollup to return/inject some fabricated module anytime something tries to import their special helpers name:
https://github.com/rollup/plugins/blob/4e85ed78cd2e941107fdf0e8e118e7bee550109d/packages/commonjs/src/index.js#L240-L243
lucaelin commentedon Sep 30, 2022
I managed to monkey-patch the commonjs module to now properly resolve the
commonjsHelpers.jsfile. The Issue is that newer versions of the commonjs-plugin inject an auxiliary plugin at the start of rollups plugin list (see here).Modifying the plugin-list during the
options-hook and returning it is not supported by the rollup-adapter as it does not handle the return value here.So everything works now? Sadly no. We are back where we started with the error message being
does not provide an export named 'exports'. 🤷lucaelin commentedon Oct 1, 2022
Trying to minimally reproduce the issue I had above, I accidentally managed to reproduce the infinite loop problem first.
The commonjs-plugins
resolveIdcalls rollups internalresolvefunction with the optionskipSelfwhich according to rollup documentation prevents infinite loops. This flag seems to not be handled properly here, as it gets ignored if the plugin provides a customresolveImporthook. I don't understand the motive for adding the exception, so I am unsure how to handle it.Edit: I now also realize that the issue seems to involve the node-resolve functionality, as the infinite loops remains when filtering out the commonjs plugin from the resolve function
Edit: Looking at the rollup implementation, I figured that each plugin that calls resolve with the skipSelf flag needs to be saved into a set and excluded from loop. Without doing this, node-import and commonjs play a very advanced game of ping-pong.
lucaelin commentedon Oct 2, 2022
I now found a solution for the third issue, regarding
does not provide an export named 'exports'. I am not 100% sure but it appears to be an issue in the commonjs plugin itself, as the module stub it imports here does not actually contain the expected export here. Since it works just fine in rollup, I am not sure how to proceed with this.All in all I created a set of monkey-patches you can load into the web-dev-server config and released them here.
This should fix all three issues at play, please give it a try and provide some feedback, so I know that my assumptions are correct.
I hope to soon create a PR for this project and start a discussion on how to fix the remaining issue in the commonjs plugin.
43081j commentedon Oct 3, 2022
@lucaelin maybe several issues at once.
the error about exports is unlikely to be from the helpers module.
https://github.com/rollup/plugins/blob/4e85ed78cd2e941107fdf0e8e118e7bee550109d/packages/commonjs/src/index.js#L241-L243
this doesn't load because the node-resolve module executes first in WTR/dev-server and decides the import is invalid (unresolvable) before we even reach the commonjs plugin code. i guess thats why they prepend the plugin you mentioned, to reach it first.
seems we would have to fix plugin-commonjs since it is producing nonsensical code (the non-existent export).
lucaelin commentedon Oct 3, 2022
Yes, it's three issues and they are all interconnected. Afaict the non-existing export is the only one within the commonjs plugin itself, it just doesn't seem to surface when bundling with rollup. I've reported that one here.
All three Issues have a workaround using the monkey-patch.
I don't think that this is true, because then my patch would not work. The
commonjsHelpers.jsimport is also prefixed with an invisible null-byte, telling node-resolve to skip resolving it and handing it over to the commonjs plugin.43081j commentedon Oct 3, 2022
yeah sorry, i meant the rollupAdapter.
it goes unresolved so reaches this
So is it basically:
?? rollupInputOptionsis meaningless (since it isn't a call to anything and isn't assigned to anything), so my guess is the original author meant to assign it but didn't@rollup/plugin-commonjson their end since the import/export don't match up and clearly should (unless somehow we're not looking at the right import and/or export, i.e. the right pair)skipSelf- weirdly this does look like it should already work, would be good to know for sure why it doesn't24 remaining items
tannerstern commentedon Feb 15, 2023
I just cloned @43081j's fork and switched to his PR branch (from #2050) and copied the built version of
dev-server-rollupit into mynode_modulesfolder and voila, it works!@akrawitz thanks for putting me on to this! I see it was marked "Ready for Review" just an hour ago, excited for it to be included in an upcoming version!
Westbrook commentedon Feb 16, 2023
For those of you interested, we're doing some testing against a canary release in this area via:
Any feedback on it working or not would be great!
tannerstern commentedon Feb 16, 2023
I added the new version that I needed with:
It's working great so far, thanks y'all!
KonnorRogers commentedon Feb 20, 2023
@Westbrook +1 here as well. Upgrading to canary version fixed the
cannot import “commonHelpers.js”error.akrawitz commentedon Feb 24, 2023
@Westbrook I can confirm that the canary version is working for me as well, but only if I also apply the fix for Windows environments in #2103.
justinfagnani commentedon Feb 24, 2023
@Westbrook I see
@web/test-runner-core@0.0.0-canary-20230216020153but not@web/test-runner- won't this mean that@web/test-runnerwill pick up the"latest"release of@web/test-runner-corebecause it's version constraint of^0.10.27doesn't match the canary version? I think you need a canary of@web/test-runnerto pick up the canary of core, or people will need to add their npm/yarn resolutions.update build pipeline dependencies
pospi commentedon Mar 30, 2023
Still hitting the same bugs here. Test environment is:
0.0.0-canary-202302160201530.1.372.77.4-1(this is the latest 2.x release which I presume is needed while Support Rollup v3.x.x open-wc/open-wc#2504 is open)I am testing with an import of the legacy
tweetnacllibrary in my project.With
@rollup/plugin-commonjs@^18.0.0it attempts to load the following URL, fails due to a 404, and the page load request hangs without spiking the CPU.http://localhost:8000/__wds-outside-root__/6/__web-dev-server__/rollup/node-resolve:empty.js?web-dev-server-rollup-null-byte=%2Fhome%2Fpospi%2Fprojects%2Fneighbourhoods%2Fapplet-template%2Fnode_modules%2F.pnpm%2Ftweetnacl%401.0.3%2Fnode_modules%2Ftweetnacl%2F%00node-resolve%3Aempty.js
With
@rollup/plugin-commonjs@^24.0.1it attempts to load this URL and fails due to a blockedtext/htmlmimetype:http://localhost:8000/__web-dev-server__/rollup/commonjs-dynamic-modules?web-dev-server-rollup-null-byte=%00commonjs-dynamic-modules
This reports as a failure to load module source for this URL and these two others, which are presumably its immediate dependants.
http://localhost:8000/__web-dev-server__/rollup/nacl-fast.js?web-dev-server-rollup-null-byte=%00%2Fhome%2Fpospi%2Fprojects%2Fneighbourhoods%2Fapplet-template%2Fnode_modules%2F.pnpm%2Ftweetnacl%401.0.3%2Fnode_modules%2Ftweetnacl%2Fnacl-fast.js%3Fcommonjs-module
http://localhost:8000/__web-dev-server__/rollup/node-resolve:empty.js?web-dev-server-rollup-null-byte=%00%2F__web-dev-server__%2Frollup%2Fnode-resolve%3Aempty.js%3Fweb-dev-server-rollup-null-byte%3D%252Fhome%252Fpospi%252Fprojects%252Fneighbourhoods%252Fapplet-template%252Fnode_modules%252F.pnpm%252Ftweetnacl%25401.0.3%252Fnode_modules%252Ftweetnacl%252F%2500node-resolve%253Aempty.js%3Fcommonjs-proxy”
I have
in my WDS
pluginsarray.pospi commentedon Mar 30, 2023
...just re-tested with
0.0.0-canary-20230328172255and the issue persists.Westbrook commentedon Mar 30, 2023
@pospi If those actually are dependents, then you're likely not leveraging the API for the Rollup Plugin as expected. Take a look at https://github.com/modernweb-dev/web/pull/2180/files#diff-71859c8cb2f330cca436fcf08f2826397cae98029c3b795a23b4b05703d56e06R207 where our test suite outlines this. The
requireReturnsDefault: 'preferred',appears support the idea that your ESM code prefers to work with its imports as if they were ESM files, but the CJS that it imports will prefer to work with its imports as if they were CJS files. I'm not a specialist of this plugin, so you'll benefit from working with the Rollup community to confirm, but this appears to be the expected usage in this context.pospi commentedon Mar 31, 2023
Tried it as suggested (
"rollup": "^3.15.0"andrequireReturnsDefault: 'preferred') but unfortunately no change to the first failing URL output above.Do I manually need to resolve this module myself? Why is this string present literally nowhere on Github let alone here or the Rollup repos and why are the Tutanota team giving it special handling? 😂
akrawitz commentedon Mar 31, 2023
@pospi I'm not sure if this will be helpful or not, but two thoughts:
@web/dev-server-rollup@0.4.0along withlatestfor the other@web/packages. As @justinfagnani noted in a comment above [@web/dev-server]:@rollup/plugin-commonjsversion>18breaks commonjs imports #1700 (comment), the dependencies for thecanaryversion may not be right if you aren't very careful.@rollup/plugin-commonjs@^18.0.0- it only became an issue with more recent versions. So, if you are seeing your problem even with@rollup/plugin-commonjs@^18.0.0, then I suspect that the underlying cause of your issue may be different.pospi commentedon Apr 11, 2023
Does that mean there's a different existing issue I should be filing these logs under, or should I submit a new one?
Currently still encountering the same failure as per
^24.0.1above. Dependencies are:jost-s commentedon Apr 24, 2023
I'm running into this issue of
Could not resolve importtoo. I've created a simple reproduction using the NPM package "ws": https://github.com/jost-s/wds-error-reproductionnpm run startwill produce the error for multiple packages, output in the terminal.Note: When using
isomorphic-wsinstead ofwsinmain.mjs, the error can be resolved by changing the filenode_modules/isomorphic-ws/node.jsfrom commonjs notationto esm notation
philSixZero commentedon Oct 18, 2023
I think this bug might be resolved with
v25.xof@rollup/plugin-commonjsandv0.3.xof@web/dev-server.Can somebody confirm that?