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 error on Browserify #978

Closed
laistomazz opened this issue May 27, 2017 · 7 comments
Closed

Bundle error on Browserify #978

laistomazz opened this issue May 27, 2017 · 7 comments

Comments

@laistomazz
Copy link

@laistomazz laistomazz commented May 27, 2017

I was trying to use require Chai while using Mocha/Karma.

ERROR [framework.browserify]: bundle error
ERROR [framework.browserify]: Error: Cannot find module './chai/config' from 'C:\folder\node_modules\chai'

The error repeats until './chai/interface/expect'

I'm using Windows 7, it worked when I switched to version 3.5.0.

@meeber
Copy link
Contributor

@meeber meeber commented May 27, 2017

@laistomazz Thanks for the issue. Any chance you could create a small, standalone repo demonstrating the error?

@laistomazz
Copy link
Author

@laistomazz laistomazz commented May 29, 2017

Here it is:
https://github.com/laistomazz/chaiIssue

I tested it on a MacOS right now and the error is the same.

I hope it helps :)

@kt3k
Copy link

@kt3k kt3k commented May 30, 2017

I'm seeing a similar error on my repo when trying to update to v4.0.0.
kt3k/spn#63

build on circle-ci
https://circleci.com/gh/kt3k/spn/345

@hakatashi
Copy link

@hakatashi hakatashi commented May 30, 2017

This seems to be the problem of "browser" field in package.json, imported by #875.

Passing --no-browser-field to browserify option may temporarily resolve the problem. (But it may have unexpected side effects)

I personally disagree that this package should have browser field in package.json. It may cause problem like this and #981.

@lucasfcosta
Copy link
Member

@lucasfcosta lucasfcosta commented May 30, 2017

Hi @laistomazz, thanks for your issue and thanks everyone for your help.

I have just read the browser field spec and I agree with @hakatashi.

Apparently it's goal is to replace the entry-point for when bundling a module, but since webpack and browserify will already go through all the requires recursively, I think we don't need that, we could just let our users bundle it the way they find to be better.

The ones wanting to use Chai in the browser can still link to the chai.js file in the root of this repo.

@meeber
Copy link
Contributor

@meeber meeber commented May 31, 2017

I've reviewed the browser field spec as well, along with @laistomazz's sample repo. I agree with the conclusions drawn in this post:

  • Problem was introduced by adding the browser field to package.json in #875.
  • The browser field isn't intended to point to a pre-bundled file; instead, it's intended to provide guidance to bundlers on how to bundle modules that have different dependencies based on server or browser environments.
  • The browser field in Chai is instructing bundlers such as Browserify and Webpack to use our browserified bundle ./chai.js as their entry point.
  • This results in bundlers attempting to bundle an already bundled file, which doesn't work.

I think the thing to do in the short-term is revert #875 and release 4.0.1 as bug fix release. In the long term, if the problem that led to #875 is still a problem, it may be possible to use the browser field in a different way to resolve that problem with Buffer. But that's for a different PR.

@keithamus
Copy link
Member

@keithamus keithamus commented May 31, 2017

Sorry, this was down to me. Should have known better. Let's release a 4.0.1 without the offending field 😄

keithamus added a commit that referenced this issue May 31, 2017
This removes the browser field which was used incorrectly. It is not needed for chai, and bundlers should bundle the commonjs code instead.
lucasfcosta added a commit that referenced this issue May 31, 2017
This removes the browser field which was used incorrectly. It is not needed for chai, and bundlers should bundle the commonjs code instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.