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

Can't run sinon in node when webpacked #177

Closed
altano opened this Issue Feb 22, 2014 · 42 comments

Comments

Projects
None yet
@altano

altano commented Feb 22, 2014

webpack is converting this:

    var isNode = typeof module !== "undefined" && module.exports;
    var isAMD = typeof define === 'function' && typeof define.amd === 'object' && define.amd;

    if (isAMD) {
        define(function(){
            return sinon;
        });
    } else if (isNode) {
        ...
    }

into this:

    var isNode = typeof module !== "undefined" && module.exports;
    var isAMD = 'function' === 'function' && typeof (require(11)) === 'object' && (require(11));

    if (isAMD) {
        {var __WEBPACK_AMD_DEFINE_RESULT__ = (function(){
            return sinon;
        }(require, exports, module)); if(__WEBPACK_AMD_DEFINE_RESULT__ !== undefined) module.exports = __WEBPACK_AMD_DEFINE_RESULT__;};
    } else if (isNode) {
        ...
    }

and '11' refers to some webpack code which is definitely defined.

Is there a way to make webpack not load a module in such a way that define.amd gets converted to something that is defined?

@altano

This comment has been minimized.

Show comment
Hide comment
@altano

altano Feb 22, 2014

I should qualify this by saying that this appears to be sinon's issue: the object returned by the AMD code branch doesn't have the same methods defined on it (not the complete API) and I'm not seeing how to configure that.

Obviously both would work with webpack if they were just returning the same object in both branches. I'll keep reading the sinon code but it's pretty obtuse.

altano commented Feb 22, 2014

I should qualify this by saying that this appears to be sinon's issue: the object returned by the AMD code branch doesn't have the same methods defined on it (not the complete API) and I'm not seeing how to configure that.

Obviously both would work with webpack if they were just returning the same object in both branches. I'll keep reading the sinon code but it's pretty obtuse.

@altano

This comment has been minimized.

Show comment
Hide comment
@altano

altano Feb 22, 2014

I filed an issue with SinonJS (sinonjs/sinon#416) but in the mean time I worked around the problem by shimming the module to disable amd:

var sinon = require("imports?define=>false!sinon");

altano commented Feb 22, 2014

I filed an issue with SinonJS (sinonjs/sinon#416) but in the mean time I worked around the problem by shimming the module to disable amd:

var sinon = require("imports?define=>false!sinon");

@altano altano closed this Feb 22, 2014

@altano

This comment has been minimized.

Show comment
Hide comment
@altano

altano Feb 22, 2014

And in case anyone stumbles on this in the future, you want to work around such issues in your webpack configuration, both because the library itself my require itself (such as SinonJS does) or some other library might require it, and you want all of them to require the same AMD-undefined version of sinon. So in my configuration I have this:

    webpack: {
      options: {
        module: {
          loaders: [
            { test: /sinon.js$/,                loader: "imports?define=>false" }
          ]
        },

K, I'll stop talking to myself now :)

altano commented Feb 22, 2014

And in case anyone stumbles on this in the future, you want to work around such issues in your webpack configuration, both because the library itself my require itself (such as SinonJS does) or some other library might require it, and you want all of them to require the same AMD-undefined version of sinon. So in my configuration I have this:

    webpack: {
      options: {
        module: {
          loaders: [
            { test: /sinon.js$/,                loader: "imports?define=>false" }
          ]
        },

K, I'll stop talking to myself now :)

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Feb 24, 2014

Member

Thx for your investigation. I once had troubles with sinon-chai.

Member

jhnns commented Feb 24, 2014

Thx for your investigation. I once had troubles with sinon-chai.

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 7, 2014

Member

AMD is just stupid....

Member

jhnns commented Mar 7, 2014

AMD is just stupid....

@altano

This comment has been minimized.

Show comment
Hide comment
@altano

altano Mar 8, 2014

I completely agree. Does anyone use it?

altano commented Mar 8, 2014

I completely agree. Does anyone use it?

@jbaiter

This comment has been minimized.

Show comment
Hide comment
@jbaiter

jbaiter May 13, 2014

The shimming-workaround doesn't do it for me:

Uncaught TypeError: Cannot read property 'invoke' of undefined

This is with the latest version (1.9.1), and I'm including it like this:

var sinon = require("imports?define=>false!sinon");
// ....
window.router = sinon.spy()

jbaiter commented May 13, 2014

The shimming-workaround doesn't do it for me:

Uncaught TypeError: Cannot read property 'invoke' of undefined

This is with the latest version (1.9.1), and I'm including it like this:

var sinon = require("imports?define=>false!sinon");
// ....
window.router = sinon.spy()
@simple10

This comment has been minimized.

Show comment
Hide comment
@simple10

simple10 May 29, 2014

@jbaiter I ran into the same issue when using require("imports?define=>false!sinon");

SinonJS requires itself with several circular dependencies. Requiring sinon.js requires sinon/spy.js which in turn requires sinon.js. But when spy.js requires sinon.js, it does not use the imports?define=>false shim, so you end up with an error.

To get everything to work properly, you must do two things:

  1. npm install imports-loader --save-dev
  2. Add the sinon shim to webpack config
// webpack.config.js
modules: {
  loaders: [
    {
      test: /sinon\.js$/,
      loader: "imports?define=>false"
    }
  ]
},

Then just use require('sinon'); as normal and everything should work.

Thanks to @altano for the solution above. I just missed his comment the first time and also didn't have imports-loader installed.

simple10 commented May 29, 2014

@jbaiter I ran into the same issue when using require("imports?define=>false!sinon");

SinonJS requires itself with several circular dependencies. Requiring sinon.js requires sinon/spy.js which in turn requires sinon.js. But when spy.js requires sinon.js, it does not use the imports?define=>false shim, so you end up with an error.

To get everything to work properly, you must do two things:

  1. npm install imports-loader --save-dev
  2. Add the sinon shim to webpack config
// webpack.config.js
modules: {
  loaders: [
    {
      test: /sinon\.js$/,
      loader: "imports?define=>false"
    }
  ]
},

Then just use require('sinon'); as normal and everything should work.

Thanks to @altano for the solution above. I just missed his comment the first time and also didn't have imports-loader installed.

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns May 29, 2014

Member

Yep. If there are circular dependencies the only way is using the config.

Member

jhnns commented May 29, 2014

Yep. If there are circular dependencies the only way is using the config.

@dcneiner

This comment has been minimized.

Show comment
Hide comment
@dcneiner

dcneiner commented Oct 22, 2014

@simple10 Thank you!

@divmain

This comment has been minimized.

Show comment
Hide comment
@divmain

divmain Oct 29, 2014

Just a heads-up, @simple10's method no longer works for Sinon 1.11.1.

divmain commented Oct 29, 2014

Just a heads-up, @simple10's method no longer works for Sinon 1.11.1.

@dcneiner

This comment has been minimized.

Show comment
Hide comment
@dcneiner

dcneiner Oct 29, 2014

All I had to do for the new version was account for the version number. It just wasn't matching the file name anymore.

/sinon.*\.js$/

dcneiner commented Oct 29, 2014

All I had to do for the new version was account for the version number. It just wasn't matching the file name anymore.

/sinon.*\.js$/

@divmain

This comment has been minimized.

Show comment
Hide comment
@divmain

divmain Oct 29, 2014

That didn't even occur to me. Thanks!

divmain commented Oct 29, 2014

That didn't even occur to me. Thanks!

@simple10

This comment has been minimized.

Show comment
Hide comment
@simple10

simple10 Nov 25, 2014

Sinon 1.12.1 requires lolex in util/fake_timer.js which breaks in webpack since it's a conditional require that webpack doesn't find by default.

There's probably a clever way to force webpack to include lolex in the bundle, but I couldn't find a quick solution.

Here's what I got working.

// In package.json, require customized sinon
{
    "sinon": "git+https://github.com/simple10/sinon-webpack",
}

// In webpack.config.js, make sure define is set to false for sinon loader
module.loaders: [{
  test: /sinon.*\.js$/
  loader: 'imports?define=>false'
}]

There's only one little customization in my modified sinon to make sure webpack requires lolex. Not an ideal solution, but works.

simple10 commented Nov 25, 2014

Sinon 1.12.1 requires lolex in util/fake_timer.js which breaks in webpack since it's a conditional require that webpack doesn't find by default.

There's probably a clever way to force webpack to include lolex in the bundle, but I couldn't find a quick solution.

Here's what I got working.

// In package.json, require customized sinon
{
    "sinon": "git+https://github.com/simple10/sinon-webpack",
}

// In webpack.config.js, make sure define is set to false for sinon loader
module.loaders: [{
  test: /sinon.*\.js$/
  loader: 'imports?define=>false'
}]

There's only one little customization in my modified sinon to make sure webpack requires lolex. Not an ideal solution, but works.

@sokra

This comment has been minimized.

Show comment
Hide comment
@sokra

sokra Nov 25, 2014

Member

Maybe someone should try to "fix" sinon with a PR:

Apply UMD.
Or build it with webpack to UMD 😄 .

Member

sokra commented Nov 25, 2014

Maybe someone should try to "fix" sinon with a PR:

Apply UMD.
Or build it with webpack to UMD 😄 .

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Nov 25, 2014

Member

Try adding sinon 2.0 to your project. It works for me (at least in the browser). Just add

"sinon": "git://github.com/cjohansen/Sinon.JS#sinon-2.0",

to your package.json.

Member

jhnns commented Nov 25, 2014

Try adding sinon 2.0 to your project. It works for me (at least in the browser). Just add

"sinon": "git://github.com/cjohansen/Sinon.JS#sinon-2.0",

to your package.json.

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns
Member

jhnns commented Nov 25, 2014

@dmatteo

This comment has been minimized.

Show comment
Hide comment
@dmatteo

dmatteo Feb 27, 2015

@jhnns +1 for that

Also a wise idea to tie it up to a specific commit, since can be unstable until release

"sinon": "git://github.com/cjohansen/Sinon.JS#b672042043517b9f84e14ed0fb8265126168778a"

dmatteo commented Feb 27, 2015

@jhnns +1 for that

Also a wise idea to tie it up to a specific commit, since can be unstable until release

"sinon": "git://github.com/cjohansen/Sinon.JS#b672042043517b9f84e14ed0fb8265126168778a"
@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 2, 2015

Member

Yup 👍

Member

jhnns commented Mar 2, 2015

Yup 👍

toolness added a commit to mozilla/learning.mozilla.org that referenced this issue Mar 11, 2015

Use sinon for stubbing in browser tests.
Note that we're using a recent sinon commit from github, as the existing
sinon on npm has issues with webpack:

webpack/webpack#177

(I would include this as a comment in package.json, but JSON doesn't
allow comments.)

vazquez pushed a commit to vazquez/teach.webmaker.org that referenced this issue Mar 12, 2015

Use sinon for stubbing in browser tests.
Note that we're using a recent sinon commit from github, as the existing
sinon on npm has issues with webpack:

webpack/webpack#177

(I would include this as a comment in package.json, but JSON doesn't
allow comments.)
@jacek-dargiel

This comment has been minimized.

Show comment
Hide comment
@jacek-dargiel

jacek-dargiel Mar 26, 2015

@jhnns @dmatteo Did you try to use sinon.fakeServer with the sinon-2.0 branch? It's not working for me (typeof sinon.fakeServer === undefined)

jacek-dargiel commented Mar 26, 2015

@jhnns @dmatteo Did you try to use sinon.fakeServer with the sinon-2.0 branch? It's not working for me (typeof sinon.fakeServer === undefined)

@jhnns

This comment has been minimized.

Show comment
Hide comment
@jhnns

jhnns Mar 27, 2015

Member

Nope, haven't tested so far

Member

jhnns commented Mar 27, 2015

Nope, haven't tested so far

@dmatteo

This comment has been minimized.

Show comment
Hide comment
@dmatteo

dmatteo Mar 27, 2015

Me neither sorry

dmatteo commented Mar 27, 2015

Me neither sorry

@iam4x

This comment has been minimized.

Show comment
Hide comment
@iam4x

iam4x Apr 3, 2015

@jacek-dargiel @jhnns @dmatteo Any new info about sinon.fakeServer? Have you find a replacement?

iam4x commented Apr 3, 2015

@jacek-dargiel @jhnns @dmatteo Any new info about sinon.fakeServer? Have you find a replacement?

@jacek-dargiel

This comment has been minimized.

Show comment
Hide comment
@jacek-dargiel

jacek-dargiel Apr 3, 2015

@iam4x To some extent - I've used karma-sinon-chai with karma.

jacek-dargiel commented Apr 3, 2015

@iam4x To some extent - I've used karma-sinon-chai with karma.

@iam4x

This comment has been minimized.

Show comment
Hide comment
@iam4x

iam4x Apr 3, 2015

Oh! I'm so dumb, forgot about the karma plugins 👍

iam4x commented Apr 3, 2015

Oh! I'm so dumb, forgot about the karma plugins 👍

@tomatau

This comment has been minimized.

Show comment
Hide comment
@tomatau

tomatau Apr 3, 2015

Is there any progress going to be made moving sinon-2-0 into the master branch? Also I can see the browserify branch... what's the value of each of these? The browserify seems to be the most recently updated.

tomatau commented Apr 3, 2015

Is there any progress going to be made moving sinon-2-0 into the master branch? Also I can see the browserify branch... what's the value of each of these? The browserify seems to be the most recently updated.

@altano

This comment has been minimized.

Show comment
Hide comment
@altano

altano Apr 4, 2015

@iam4x I use https://github.com/moll/node-mitm instead of sinon for dealing with XHR mocking. It was super easy, not at all flaky, and works perfectly with webpack.

altano commented Apr 4, 2015

@iam4x I use https://github.com/moll/node-mitm instead of sinon for dealing with XHR mocking. It was super easy, not at all flaky, and works perfectly with webpack.

@iam4x

This comment has been minimized.

Show comment
Hide comment
@iam4x

iam4x Apr 4, 2015

Nice alternative @altano thank's

iam4x commented Apr 4, 2015

Nice alternative @altano thank's

@srph

This comment has been minimized.

Show comment
Hide comment
@srph

srph Apr 9, 2015

Thanks @simple10 👍

srph commented Apr 9, 2015

Thanks @simple10 👍

@atondelier

This comment has been minimized.

Show comment
Hide comment
@atondelier

atondelier Oct 28, 2015

I encountered the same issue: sinon doesn't export itself correctly when webpacked.


Solution without karma

I first ended up with this solution.

Add the following imports loader configuration in webpack configuration loaders array:

{
    test: /sinon\.js$/,
    loader: "imports?define=>false,require=>false"
}

Notice the addition of require=>false to @simple10 's solution.
Then sinon can be required using the built package:

const sinon = require('sinon/pkg/sinon');

You are free to add an alias from "sinon" to "sinon/pkg/sinon" in your test webpack configuration.


Solution with karma

My case was browser-side, so packages like karma-sinon-chai which can make the browser load the built file of sinon are a solution. Sinon is not part of the test webpacked bundle file any more.

atondelier commented Oct 28, 2015

I encountered the same issue: sinon doesn't export itself correctly when webpacked.


Solution without karma

I first ended up with this solution.

Add the following imports loader configuration in webpack configuration loaders array:

{
    test: /sinon\.js$/,
    loader: "imports?define=>false,require=>false"
}

Notice the addition of require=>false to @simple10 's solution.
Then sinon can be required using the built package:

const sinon = require('sinon/pkg/sinon');

You are free to add an alias from "sinon" to "sinon/pkg/sinon" in your test webpack configuration.


Solution with karma

My case was browser-side, so packages like karma-sinon-chai which can make the browser load the built file of sinon are a solution. Sinon is not part of the test webpacked bundle file any more.

@trodrigues

This comment has been minimized.

Show comment
Hide comment
@trodrigues

trodrigues Feb 18, 2016

I landed here and after trying all the solutions and having none of them worked, I figured out that sinon 2 fixes this. It's currently still on npm as 2.0.0-pre, so just use that and it'll work fine. Also, @altano maybe worth editing your original comment with a note on that :)

trodrigues commented Feb 18, 2016

I landed here and after trying all the solutions and having none of them worked, I figured out that sinon 2 fixes this. It's currently still on npm as 2.0.0-pre, so just use that and it'll work fine. Also, @altano maybe worth editing your original comment with a note on that :)

@wwalser

This comment has been minimized.

Show comment
Hide comment
@wwalser

wwalser Feb 18, 2016

Thanks @trodrigues, just hours apart but you saved me loads of time.

wwalser commented Feb 18, 2016

Thanks @trodrigues, just hours apart but you saved me loads of time.

joshdmiller added a commit to StoryShop/app that referenced this issue Feb 22, 2016

refactor: add resolve.root to simplify requires
- use webpack's resolve.root to allow resolving from src;
- move utils folder to src to take advantage of above;
- change webpack and npm scripts to compile specs before running specs, to accommodate above;
- remove sinon in favour of homebrewed spy functionality, because sinon cannot work with webpack
  (webpack/webpack#177);
- change all applicable relative paths to resolved paths;
- change wdio pauses to 50ms from 1000ms to make tests run faster.
@danielbush

This comment has been minimized.

Show comment
Hide comment
@danielbush

danielbush Feb 28, 2016

Has anyone tried to use sinon-as-promised with any of the above solutions?
If I'm using sinon 1, I have to import/require sinon/pkg/sinon in my code and use loader: "imports?define=>false,require=>false" in my webpack config (with test: /sinon\.js/). But sinon-as-promised uses require('sinon') and falls over. If I reach into the code and change it to use sinon/pkg/sinon like my tests, it works.

I've tried changing the webpack test from /sinon\.js/ to /sinon.*\.js/ to see if I can get requiring 'sinon' to work instead of falling back to 'sinon/pkg/sinon', but didn't get anywhere with it and had to exclude things like sinon-chai etc.

If I switch to sinon 2.0.0-pre (and remove the import loader hack and use of sinon/pkg/sinon), sinon-as-promised gets a little further but falls over for probably a compatibility reason, since it is peer-dependent with sinon 1.

Sad face. Another js session lost to tooling.

danielbush commented Feb 28, 2016

Has anyone tried to use sinon-as-promised with any of the above solutions?
If I'm using sinon 1, I have to import/require sinon/pkg/sinon in my code and use loader: "imports?define=>false,require=>false" in my webpack config (with test: /sinon\.js/). But sinon-as-promised uses require('sinon') and falls over. If I reach into the code and change it to use sinon/pkg/sinon like my tests, it works.

I've tried changing the webpack test from /sinon\.js/ to /sinon.*\.js/ to see if I can get requiring 'sinon' to work instead of falling back to 'sinon/pkg/sinon', but didn't get anywhere with it and had to exclude things like sinon-chai etc.

If I switch to sinon 2.0.0-pre (and remove the import loader hack and use of sinon/pkg/sinon), sinon-as-promised gets a little further but falls over for probably a compatibility reason, since it is peer-dependent with sinon 1.

Sad face. Another js session lost to tooling.

@srph

This comment has been minimized.

Show comment
Hide comment
@srph

srph Feb 28, 2016

@danielbush

Add an alias (resolve.alias).

{
  resolve: {
    alias: { sinon: 'sinon/pkg/sinon' } // https://github.com/webpack/webpack/issues/177#issuecomment-185718237
  }
}

srph commented Feb 28, 2016

@danielbush

Add an alias (resolve.alias).

{
  resolve: {
    alias: { sinon: 'sinon/pkg/sinon' } // https://github.com/webpack/webpack/issues/177#issuecomment-185718237
  }
}
@kadamwhite

This comment has been minimized.

Show comment
Hide comment
@kadamwhite

kadamwhite Mar 11, 2016

We ran into this issue when using Enzyme to test react components, and what worked for us (to document for posterity) was a combination of above approaches:

webpack: {
  resolve: {
    alias: {
      'sinon': 'sinon/pkg/sinon'
    }
  },
  module: {
    // don't run the sinon module through babel-loader
    noParse: [
      /sinon/
    ],
    // ...

kadamwhite commented Mar 11, 2016

We ran into this issue when using Enzyme to test react components, and what worked for us (to document for posterity) was a combination of above approaches:

webpack: {
  resolve: {
    alias: {
      'sinon': 'sinon/pkg/sinon'
    }
  },
  module: {
    // don't run the sinon module through babel-loader
    noParse: [
      /sinon/
    ],
    // ...
@ericlau-solid

This comment has been minimized.

Show comment
Hide comment
@ericlau-solid

ericlau-solid Mar 31, 2016

I have to read the entire thread to figure out what I actually need to do. To summarize this for everyone, all the webpack.config changes required to get sinon to work (don't forget to install imports-loader):

resolve: {
        alias: { sinon: 'sinon/pkg/sinon' }
    },
    loaders: [{ test: /sinon.*\.js$/,   loader: "imports?define=>false,require=>false"  }],
    noParse: [/sinon/]

ericlau-solid commented Mar 31, 2016

I have to read the entire thread to figure out what I actually need to do. To summarize this for everyone, all the webpack.config changes required to get sinon to work (don't forget to install imports-loader):

resolve: {
        alias: { sinon: 'sinon/pkg/sinon' }
    },
    loaders: [{ test: /sinon.*\.js$/,   loader: "imports?define=>false,require=>false"  }],
    noParse: [/sinon/]
@alanclarke

This comment has been minimized.

Show comment
Hide comment
@alanclarke

alanclarke Mar 31, 2016

@ericlau-solid does that work for you when calling useFakeTimers?

alanclarke commented Mar 31, 2016

@ericlau-solid does that work for you when calling useFakeTimers?

@alanclarke

This comment has been minimized.

Show comment
Hide comment
@alanclarke

alanclarke commented Mar 31, 2016

nm, just use sinon@2.0.0-pre

onishiweb added a commit to Financial-Times/o-viewport that referenced this issue May 24, 2016

Add tests for throttle and debounce functions
This also includes an update to Sinon so that the useFakeTimers
function will work with Webpack, see:
webpack/webpack#177 (comment)
@aaronjensen

This comment has been minimized.

Show comment
Hide comment
@aaronjensen

aaronjensen Jun 18, 2016

fwiw, we had better luck using:

import 'expose?lolex!lolex'
import 'script!sinon/pkg/sinon'

(replace with require/webpack aliases/loader config/etc if you want)

aaronjensen commented Jun 18, 2016

fwiw, we had better luck using:

import 'expose?lolex!lolex'
import 'script!sinon/pkg/sinon'

(replace with require/webpack aliases/loader config/etc if you want)

@ZebraFlesh

This comment has been minimized.

Show comment
Hide comment
@ZebraFlesh

ZebraFlesh Aug 20, 2016

By far, the easiest solution is to use sinon@2.0.0-pre. I tested with sinon@2.0.0-pre.2 and sinon.useFakeXMLHttpRequest, worked great.

ZebraFlesh commented Aug 20, 2016

By far, the easiest solution is to use sinon@2.0.0-pre. I tested with sinon@2.0.0-pre.2 and sinon.useFakeXMLHttpRequest, worked great.

@renminghao

This comment has been minimized.

Show comment
Hide comment

renminghao commented Aug 24, 2016

@atondelier thanx

EvilJimJafar added a commit to EvilJimJafar/react-redux-mocking that referenced this issue Sep 11, 2016

@Lexicality

This comment has been minimized.

Show comment
Hide comment
@Lexicality

Lexicality Sep 14, 2016

To prevent imports-loader clobbering sinon-chai, I had to change the loader config to

{
    test: /sinon\/pkg\/sinon/,
    loader: "imports?define=>false,require=>false"
}

Lexicality commented Sep 14, 2016

To prevent imports-loader clobbering sinon-chai, I had to change the loader config to

{
    test: /sinon\/pkg\/sinon/,
    loader: "imports?define=>false,require=>false"
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment