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

Karma + Karma-bro + proxyquireify don't get along #15

Closed
xcambar opened this issue Sep 16, 2014 · 19 comments
Closed

Karma + Karma-bro + proxyquireify don't get along #15

xcambar opened this issue Sep 16, 2014 · 19 comments

Comments

@xcambar
Copy link

xcambar commented Sep 16, 2014

Hi,
I've been struggling for a day with trying to have proxyquireify in Karma with Karma-bro.

In a few words: Proxyquireify adds the authentic require calls so Browserify bundles them, but it seems that the prelude is not changed so the mocked modules are not loaded.

Here's a example repo: https://github.com/xcambar/karma-bro-proxyquireify

Did I do something wrong? What do you think?

/cc @nikku because I feel like Karma-bro might be involved in this situation.

@thlorenz
Copy link
Owner

It looks ok to me except I don't see where you register it as a plugin with browserify, i.e. like here

@xcambar
Copy link
Author

xcambar commented Sep 16, 2014

here, in the Karma conf

@thlorenz
Copy link
Owner

Ok, so as long as this plugin gets added to browserify and karma doesn't mess with the prelude and/or the require function things should work.

You could trouble shoot this by debugging the tests and/or reading the relevant parts of your bundle.

@xcambar
Copy link
Author

xcambar commented Sep 17, 2014

https://github.com/thlorenz/proxyquireify/blob/master/index.js#L70-L72 sets the _stubs and cache then resets right after.

https://github.com/thlorenz/proxyquireify/blob/master/index.js#L71 uses the original require. Shouldn't it call proxyquire.proxy (https://github.com/thlorenz/proxyquireify/blob/master/index.js#L81) ?

I'll test it and PR if relevant.

@xcambar
Copy link
Author

xcambar commented Sep 17, 2014

For greater details, here's my complete bundle (proxyquireify with the aforementioned changes): https://gist.github.com/xcambar/906c17cdf6fb8a8bdbdc

There's obviously something missing, but for the moment, I can't figure out what.

@thlorenz
Copy link
Owner

I don't have time to look at this right now. Hopefully someone else can help you figure this out in the meantime, maybe @jhiesey?

@ellbee
Copy link

ellbee commented Sep 22, 2014

I think the problem is caused by the reset method being called on the browserify instance, which creates a new pipeline that doesn't have the proxyquireify replacement prelude spliced in. This changes made in this commit seem to pass your test: ellbee@fdbb2da

@daegren
Copy link

daegren commented Sep 22, 2014

I am also getting the same issue when trying to rebundle with proxyquireify using watchify, however @ellbee's fix just seems to hang on my pipeline. I've put up a small test repo to test this. https://github.com/daegren/proxyquireify-test

@ellbee
Copy link

ellbee commented Sep 22, 2014

Your test repo works for me with the changes from my commit if I move the plugin registration out from the bundle function, so it only registers it once. If I leave the plugin registration inside the plugin function I get a gulp error on rebundle.

@daegren
Copy link

daegren commented Sep 22, 2014

Thanks @ellbee! That worked, much appreciated.

@thlorenz
Copy link
Owner

@ellbee is there any patch we can pull in here? Do you mind providing a PR? I'm happy to pull this in as long as it doesn't break current behavior.

@ellbee
Copy link

ellbee commented Sep 23, 2014

@daegren Cool, glad that helped.

@thlorenz I've created a pull request #17

Edit: I changed the pull request slightly, so that it now only adds the transform when the plugin is registered and not every time reset is called.

@xcambar
Copy link
Author

xcambar commented Sep 23, 2014

Awesome !

@beckyconning
Copy link

is there any way to use these changes in order to swap between a proxyquire wrapper and proxyquireify when karma-bro browserifies tests?

i'm in a situation where this:

var proxyquire = require('./proxyquire-wrapper')(require);

works in node and this

var proxyquire = require('proxyquireify')(require);

works in the browser but i can't seem to make karma-bro swap the wrapper for proxyquireify for me.

thanks!

@beckyconning
Copy link

I use karma-bro with proxyquireify through https://github.com/bendrucker/proxyquire-universal.

You can use it with karma-bro by simply adding this to your karma configuration.

browserify: { debug: true, plugin: ['proxyquire-universal'] }

@xcambar
Copy link
Author

xcambar commented Oct 21, 2014

Very cool @beckyconning. Thanks a lot!

@bendrucker
Copy link
Collaborator

This is the same as #21 and there's a long discussion there. Turned out it was a one line fix, merging in #22.

@K4LiN
Copy link

K4LiN commented Jan 19, 2018

I've got exactly the same issue with the current versions of proxyquire:
"proxyquire-universal": "^1.0.8",
"proxyquireify": "^3.2.1",

I checked the @xcambar git repo and applying patch or updating the proxyquirefy to current isn't solving the issue.

Ps. sorry for digging up the thread ;)

@bendrucker
Copy link
Collaborator

bendrucker commented Jan 19, 2018

Because this issue is pretty ancient, I have to ask that you open a new one if you think you've found a legitimate bug. This is in large part in fairness to the original contributors to the discussion who shouldn't be forced to unsubscribe from notifications on a 3 year old thread.

It's possible a major version of browserify or karma* broke something. To start we'll need a project (public repo) that we can clone, run npm install && npm test, and get exit 1 with a trace of the error you're seeing. As few deps as possible, e.g. use assert and not something like mocha.

Repository owner locked and limited conversation to collaborators Jan 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants