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

Broken build with rollup for node due to global "navigator" #113

Closed
arctica opened this issue Dec 13, 2020 · 6 comments
Closed

Broken build with rollup for node due to global "navigator" #113

arctica opened this issue Dec 13, 2020 · 6 comments

Comments

@arctica
Copy link

arctica commented Dec 13, 2020

Mergerequest #111 broke our build via rollup for nodejs because it assumes a global "navigator" exists.

It does not seem rollup can handle the "exports" part of the package.json and therefor includes browser.js.

I'm not knowledgeable enough to know what the proper fix would be but I can imagine changes to package.json or a check for "navigator" being undefined would do the trick.

@Qix-
Copy link
Member

Qix- commented Dec 13, 2020

Please open an issue with Rollup. browser.js should not be packaged if you're not targeting the browser. As far as I can see, our package is set up properly to handle the different targets.

@Qix- Qix- closed this as completed Dec 13, 2020
@arctica
Copy link
Author

arctica commented Dec 13, 2020

@Qix- as per the below two Rollup issues, it seems like they have some real problems with how "exports" was added to package.json and don't seem to want to support it anytime soon.

  1. Support "exports" field in package.json rollup/plugins#208
  2. Support Dual CommonJS/ES Module Packages with isolated state rollup/rollup#3514

Especially this comment: rollup/rollup#3514 (comment)

So seems like unless one is using webpack or node directly, the current package.json wont work when bundling and then using in node.

Please consider adding a typeof navigator !== 'undefined' check to const matches = /(Chrome|Chromium)\/(?<chromeVersion>\d+)\./.exec(navigator.userAgent);. The vast majority of libs I inspected do something similar.

@Qix-
Copy link
Member

Qix- commented Dec 13, 2020

@sindresorhus Can you make sense of this? The package.json nonsense is becoming a huge unmaintainable mess. Every few weeks there's a new key in the package.json and a new maintainer that's on a pedestal claiming there's some new fancy way their bundler has created to include ESM/Browser/Node frankenstein project configurations in one package and it's becoming completely un-followable at this point. I'm seeing it over at debug more and more now, too.

What is the definitive way to do this? This is ridiculous at this point - it's been years and npm clearly doesn't want to listen to the OSS community calling them out about it being a mess.

But we keep getting package.json-related issues like clockwork every couple of months it seems and there's some completely new way to do packaging. We do method A, break a bunch of people, so switch to a "better" method B and break the previously supported people, and then a new bundler comes out and all of a sudden there's a new barrage of package.json configs and keys and arguments and debates. It's getting old.

What is going on?

@arctica
Copy link
Author

arctica commented Dec 14, 2020

Hehe I didn't want to go in this direction but yes, the whole situation with packaging in JS world to me as a non JS developer seems like a big mess. I have spent an unreasonable amount of time trying to get our project set up and running with all kinds of dependencies. The whole browser vs node, bundler X vs bundler Y, require() vs import, module.exports vs export etc incompatibility thing is a real problem. And then I can't even do a real deep update of all dependencies without npm running for 20min and then crashing :)

That's why I suggested the check for nagivator being undefined because then browser.js can be included in node projects without breaking stuff. But I can also understand why a package author would be against that as they should rightfully be able to expect this package.json stuff to do the right thing. I mean it's a simple job of "include this file in this environment or not".

In the end the package authors, app developers and end users are the ones who are having to deal with the mess.

@sindresorhus
Copy link
Member

Doing workarounds for bundlers is an endless slippery slope. I've done it a lot in the past and it always opens the flood gates for more. I don't intend to add any workaround here. I would recommend trying harder to get Rollup to properly support exports in package.json. It's in Node.js and not going anywhere, so if Rollup decides not to support it, they have a big problem.

Also relevant: rollup/plugins#695

@mattfysh
Copy link

Oh dang, just ran into this issue... was super difficult to track down as the code was being built for lambda so you can imagine the hurdles needed to find out where this was going wrong.

@arctica what workaround did you use in the end? thanks!

nemo-shen added a commit to nemo-shen/vuiter that referenced this issue Aug 16, 2024
Due to rollup not support `imports` in package.json

Relate issues:

chalk: [#113](chalk/supports-color#113)
chalk: [#578](chalk/chalk#578)
rollup: [#3514](rollup/rollup#3514)
nemo-shen added a commit to nemo-shen/vuiter that referenced this issue Oct 31, 2024
Due to rollup not support `imports` in package.json

Relate issues:

chalk: [#113](chalk/supports-color#113)
chalk: [#578](chalk/chalk#578)
rollup: [#3514](rollup/rollup#3514)
nemo-shen added a commit to nemo-shen/vuiter that referenced this issue Nov 4, 2024
Due to rollup not support `imports` in package.json

Relate issues:

chalk: [#113](chalk/supports-color#113)
chalk: [#578](chalk/chalk#578)
rollup: [#3514](rollup/rollup#3514)
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

4 participants