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

Quibble in the browser #77

Closed
randycoulman opened this issue Mar 14, 2016 · 15 comments
Closed

Quibble in the browser #77

randycoulman opened this issue Mar 14, 2016 · 15 comments

Comments

@randycoulman
Copy link
Contributor

I'm trying to set up my project to run the tests in the browser for debugging purposes, using webpack-dev-server. When I do so, I run into issues with quibble. I note from f694c82 that there's been some work done to guard against requireing quibble in the browser, but there are two more unguarded references to it. One is in reset.coffee, and the other is in replace/module.coffee. I believe the former reference is causing my issue.

I'm seeing the following error message:

ERROR in ./~/quibble/lib/quibble.js
Module not found: Error: Cannot resolve module 'module' in /Users/randy/src/scratch-it/skraphaug/node_modules/quibble/lib
 @ ./~/quibble/lib/quibble.js 9:11-28

But looking at the stack trace in my browser, it looks like the require of quibble in reset.coffee is the root problem.

I'll be happy to provide any additional information or settings you need.

@searls
Copy link
Member

searls commented Mar 14, 2016

To be clear, your issue is that when you use td.js in your browser, you're getting unrelated quibble errors, right? It's not the case that you're trying to make quibble work in the browser (which isn't supported), right?

@searls
Copy link
Member

searls commented Mar 14, 2016

If so, then this is definitely a bug. What I don't understand is why our test suite hasn't caught it

@randycoulman
Copy link
Contributor Author

Yes, correct. I'm not even using the functionality required by quibble, but I can't get a clean webpack compile because of the above error.

I'm not sure why the test suite isn't catching it. For me, the error is originating where I'm calling td.reset() in my global Mocha afterEach().

@searls
Copy link
Member

searls commented Mar 14, 2016

I have an idea: could you add a skeleton webpack project to the examples/ directory with a failing npm test command so i can add that to the build?

randycoulman added a commit to randycoulman/testdouble.js that referenced this issue Mar 15, 2016
This example has a basic webpack configuration that attempts to build a
test bundle.  There’s also an `npm run test:debug` script to allow
running the tests in the browser (at `localhost:8081`).

The tests can’t be run from the command line, but this example
demonstrates the issue reported in testdouble#77.  Once that issue is resolved,
the example should build correctly, and the tests should run in the
browser.
@randycoulman
Copy link
Contributor Author

I've added the example; there's not much there, but it does illustrate the problem. I note that the fix in f694c82 doesn't actually fix the problem. If I simply do var td = require('testdouble'); in my test file (without calling td.reset()), I continue to get the same error on the command-line, but this time the root cause is in replace/index.js instead.

@searls
Copy link
Member

searls commented Mar 15, 2016

After chatting with @jasonkarns & @davemo, I think the root cause is that webpack (which I don't have experience with) will try to slurp in npm dependencies in much the same way browserify will, but without (apparently) the same level of support for Node's built-in modules or an awareness of the package.json's browser field.

Rather than have webpack attempt to rebuild an already-compiled-down-to-ES3 and browserified vendor library, wouldn't it make more sense to link to the browser distribution in dist/testdouble.js? I presume webpack must support this.

@davemo
Copy link
Contributor

davemo commented Mar 15, 2016

I was able to make it work by using the webpack exports-loader, and downloading a copy of the browser distribution of testdouble.js; relevant reading here: https://webpack.github.io/docs/shimming-modules.html

var expect = require('chai').expect;
var td = require('exports?window.td!./test/helpers/testdouble.js')

afterEach(function(){
  td.reset()
})

describe('sample Mocha test', () => {
  it('passes', () => {
    expect(true).to.be.true
  })
})

@davemo
Copy link
Contributor

davemo commented Mar 15, 2016

You'll need to install the exports-loader for this to work: https://github.com/webpack/exports-loader

@searls
Copy link
Member

searls commented Mar 15, 2016

Looks like this is the right answer. Yay package.json being a vendor specific dumping ground and not semantic! https://twitter.com/statenjason/status/709773052729368577

@davemo
Copy link
Contributor

davemo commented Mar 15, 2016

I tested this in #79, and it solves the problem; you do get the warning in the terminal and browser, but the tests execute just fine.

screen shot 2016-03-15 at 12 59 18 pm
screen shot 2016-03-15 at 12 59 28 pm

@randycoulman
Copy link
Contributor Author

1.1.1 is helping; I'm still running into a couple of issues, but I'm not sure of the root cause yet. I'll report back here once I figure out what's going on.

@jasonkarns
Copy link
Member

Looks like webpack would respect the browser field if we didn't use browserify's special "replace this with that" semantics. https://github.com/testdouble/testdouble.js/blob/master/package.json#L36

https://webpack.github.io/docs/configuration.html#resolve-packagemains

@randycoulman
Copy link
Contributor Author

The issues I'm running into are unrelated to testdouble.js, so it looks like 1.1.1 gets me what I need. I agree, though, that it would be nice to eliminate that ugly warning. Thanks for the help and research on this!

searls pushed a commit that referenced this issue Mar 16, 2016
This example has a basic webpack configuration that attempts to build a
test bundle.  There’s also an `npm run test:debug` script to allow
running the tests in the browser (at `localhost:8081`).

The tests can’t be run from the command line, but this example
demonstrates the issue reported in #77.  Once that issue is resolved,
the example should build correctly, and the tests should run in the
browser.
@searls
Copy link
Member

searls commented Mar 16, 2016

Well, this has been educational. Thanks for raising the issue and forcing me to spend far more time than I would have liked learning about webpack.

Improvements landed in v1.1.3

@randycoulman
Copy link
Contributor Author

Thanks for putting the time in to figure this out. Sorry I wasn't more help. As I suspect is the case with most people, my webpack knowledge largely consists of finding snippets on the internet that work for other people and pasting them into my config file :-)

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