jasmine-ajax
jasmine-ajax copied to clipboard
Check for a global jasmine and use it if found.
This makes jasmine-ajax work with Browserify. Browserify is a CommonJS environment, but it runs in a browser and doesn't play nicely with jasmine-core (or at least I had trouble using it in my project). The way we use Jasmine - and I suspect this isn't uncommon - is we include Jasmine and our Browserify-built bundle using regular script tags. So jasmine is global and requiring-in jasmine-core is incorrect, even though we are in a CommonJS environment.
This change only checks whether a global jasmine is defined. A slightly more defensive approach - but perhaps overkill? - would be to also check for particular properties on that jasmine object. i.e.: make sure it's the right jasmine.
(I originally opened this PR editing the lib/mock-ajax.js. Oops. It's now correctly editing src/boot.js.)
For those of us using Jasmine with Karma, this is very much breaking. fs is not available in the browser.
@toddwildey you mean the original change is breaking, and this PR fixes the breaking, right? That was my experience, at least.
No - I mean this change breaks a Jasmine + Karma setup. I fixed it by adding a "root !== window" check in src/boot.js. I would submit a PR, but for reasons I won't dive into, I'm not "allowed."
Interesting, I'm using Jasmine + Karma setup myself, and I needed this change for it to work. Maybe the difference is that my specs are bundled by Browserify and yours aren't?
Can you elaborate on how it breaks? In your setup, jasmine is defined, and yet you do want to require('jasmine-core')?
jasmine-core uses 'fs', so when jasmine-ajax includes jasmine-core, my browser is hung up on how to require 'fs' when running Karma tests.
I should add that I'm using webpack for bundling. Because of this, exports is defined, which is incurring the require('jasmine-core') call.
Are you sure you don't have that problem without this pull request? Because I think that this change will make it so that require is never actually executed at runtime. However, Webpack (both with and without this PR) will see the require('jasmine-core') and bundle it in for you, because it doesn't know any better.
We're actually in the process of switching from Browserify to Webpack, so I ran into this same problem the other day. My solution was to add code like this to webpack.config.js:
module: {
loaders: [
{
// Don't let jasmine-ajax detect require and import jasmine-core, because we bring
// in Jasmine via a script tag instead.
test: require.resolve('jasmine-ajax'),
loader: 'imports?require=>false'
}
]
}
Just to be clear, my experience was:
- Latest released version of
jasmine-ajax(v3.2.0): works great with Webpack + Jasmine + Karma - Master: broken with Webpack because webpack pulls in jasmine-core
- This PR: still broken with Webpack, for the same reason.
@toddwildey I saw you posted a comment describing a problem and then deleted it. Were you able to solve the problem?
I was not - not without modifying webpack.config.js. The error is still showing up, but it's no longer breaking my builds, since that code path never gets executed during tests. So while it may be annoying, it's no longer blocking.
Providing webpack.confg.js with an alias for 'jasmine-core' can fix the problem, but it's burdensome to consumers. I ultimately don't see a solution to this without removing the require('jasmine-core') call from the code.
I'll leave it to the jasmine-ajax maintainers to say for sure, but my understanding is that the require('jasmine-core') can't be removed because it's necessary when using the library under node.
Did you try the webpack.config.js changes I suggested above? I think that's the right way to deal with it in webpack. It's better than adding an alias for jasmine-core, anyway, because you really want jasmine-ajax to follow the other code path and add itself to the global jasmine object.
It's starting to look like we'll need to revisit how Jasmine-Core is packaged up for the various environments to make this all work. Currently the thing available via require('jasmine-core'); is assuming it is being loaded in a node.js context and thus has access to things like fs. There is also some work being attempted in Jasmine-Core around this: see jasmine/jasmine#970.
Unfortunately, I haven't really worked in a CommonJS/AMD type environment enough to have opinions on what Jasmine should do here.
Closing since this has long since been overtaken by changes in jasmine-ajax, jasmine-core, and likely the front end development ecosystem. We haven't heard anything about CommonJS in the browser in a number of years, so my guess is that there's no longer anything Jasmine needs to do here. If there is, it's probably going to require a new approach based on the current state of the world.