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

Bundle fails for form require('../#/folder') due to hashbang-only directory #528

Closed
ctjlewis opened this issue Aug 4, 2020 · 8 comments
Closed

Comments

@ctjlewis
Copy link
Contributor

ctjlewis commented Aug 4, 2020

  • Rollup Plugin Name: commonjs
  • Rollup Plugin Version: 14.0.0
  • Rollup Version: 1.27.14
  • Operating System (or Browser): Ubuntu
  • Node Version: v12.18.2
  • Link to reproduction (⚠️ read below):
    https://repl.it/@christiantjl/rollup-hashbang-dir (use yarn test for repro)

Expected Behavior

Bundle produced at output/bundle.mjs.

Actual Behavior

Error is thrown, presumably due to hashbang-only directory in filepath and/or index.js assumption (see below).

Additional Information

I'm working to stress test Rollup against the entire dependency tree of the top 100 or so npm packages, which involves loading a set of plugins for legacy compatibility: commonjs, node-resolve, and json. That work will be ongoing in a separate repo, but I wanted to submit this bug report in advance since it was the most common error at compile-time (the stress test will also run the bundle through node runtime).

import statements for vast majority of these 1300 or so packages will indeed bundle, but a few (~17) do not, ~75% of which are modules that rely on a popular package called es5-ext. Some packages which cannot bundle because of this error include: d gulp es6-iterator es6-symbol es6-weak-map gulp gulp-cli last-run semver-greatest-satisfied-range sver-compat undertaker

Consider the example module on repl.it, that just imports this package to demonstrate:

import hashbangDirInDepTree from 'es5-ext';

Bundling with the plugins I mentioned throws:

index.mjs → output/bundle.mjs...
[!] (plugin Rollup Core) Error: Could not load /home/runner/rollup-hashbang-dir/node_modules/es5-ext/array/index.js#/compact (imported by /home/runner/rollup-hashbang-dir/node_modules/es5-ext/json/safe-stringify.js): ENOENT: no such file or directory, open '/home/runner/rollup-hashbang-dir/node_modules/es5-ext/array/index.js#/compact'

Notice how it attempts to resolve to array/index.js#/compact and not array/#/compact/index.js for what I assume is https://github.com/medikoo/es5-ext/blob/master/json/safe-stringify.js#L3 :

var compact  = require("../array/#/compact")

Please let me know if I have been unclear or can otherwise provide more information. The error message indicates Rollup Core, but I believe it's the CommonJS plugin that is directing Core to load a nonexistent file so I opened the issue in this repo.

@ctjlewis ctjlewis changed the title CommonJS plugin fails for form require('../#/folder') due to hashbang-only directory [commonjs] Bundle fails for form require('../#/folder') due to hashbang-only directory Aug 4, 2020
@UnbearableBear
Copy link

UnbearableBear commented Aug 5, 2020

I might be wrong but it seems that this issue is prior to v14.0.0

We have the exact same issue in our repo, I confirm that things such as:

var contains = require("es5-ext/string/#/contains");

like here (https://github.com/medikoo/d/blob/master/index.js#L7)

are, I assume, wrongly converted to this path:
es5-ext/string/index.js#/contains

Maybe we should be able to provide an option to allow this kind of char in the require path to be considered as part of the actual path ?

@ctjlewis
Copy link
Contributor Author

ctjlewis commented Aug 5, 2020

@UnbearableBear I found this during a stress test, so it's interesting that this actually impacted real-life code; es5-ext is a super common dependency, so it's not that surprising I guess.

I will try to work on a fix soon. Smarter people than me like @Andarist or @lukastaegert might have some advice on where to start regarding this issue.

@UnbearableBear
Copy link

Thanks a lot ! I'll try to cast an eye over it too but I'm going on holidays tomorrow so that's going to be shhhort

@Andarist
Copy link
Member

Andarist commented Aug 6, 2020

Well, I'm not aware of any special use cases for # in the path - are there maybe some tests for this? If we could establish that there are no special use cases for # then I would stay that the resolving logic should be tweaked to just consider # a part of the actual fs path.

@shellscape shellscape changed the title [commonjs] Bundle fails for form require('../#/folder') due to hashbang-only directory Bundle fails for form require('../#/folder') due to hashbang-only directory Sep 6, 2020
@SCWR
Copy link

SCWR commented Sep 10, 2020

I have the same problem when I use css related plugins,

rollup-plugin-css-porter
rollup-plugin-embed-css

@stale stale bot added the x⁷ ⋅ stale label Nov 9, 2020
@stale
Copy link

stale bot commented Nov 10, 2020

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.

@stale stale bot closed this as completed Nov 10, 2020
@ctjlewis
Copy link
Contributor Author

Wasn't this addressed in a recent release?

@Emptymu
Copy link

Emptymu commented Jul 7, 2021

For anyone having this problem, update your @rollup/plugin-node-resolve.
Here is the MR that fixed it:
#588

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

6 participants