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

Browser bundling regression #18

Closed
porsager opened this issue Dec 31, 2017 · 3 comments
Closed

Browser bundling regression #18

porsager opened this issue Dec 31, 2017 · 3 comments

Comments

@porsager
Copy link

porsager commented Dec 31, 2017

Bundling object-inspect with rollup doesn't work and will throw errors because it thinks the package relies on node's util module.

var inspectCustom = require('./util.inspect')

and there

module.exports = require('util').inspect;

Relevant commit where the change was made.
20cca77

@ljharb
Copy link
Member

ljharb commented Dec 31, 2017

That commit explicitly added a browser entry point with the "browser" field in package.json.

Does rollup not support that community standard? Browserify and webpack don't seem to have trouble with it (I tested that explicitly prior to publishing).

@ljharb
Copy link
Member

ljharb commented May 2, 2018

I'm going to close this; please file an issue on rollup if it can't handle the "browser" field and link it here!

@ljharb ljharb closed this as completed May 2, 2018
@porsager
Copy link
Author

Sorry for the late answer @ljharb ..

Seems like there's finally a PR to add support on rollup here. rollup/rollup-plugin-node-resolve#183

porsager added a commit to porsager/rollup-plugin-node-resolve that referenced this issue Jan 21, 2019
It's pretty common for node modules to use require() without specifying the file extension, so adding this will most likely make a few more libraries work with rollup. One project doing this is for instance `object-assign` which doesn't work currently. inspect-js/object-inspect#18

Considering the line that checks for changed paths below does exactly the same it's probably just an oversight that this was missing.
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

2 participants