Comment author: dmethvin

Since this is a regression, I'd like to fix it. However, we don't unit test or support jasmine-node so to have any chance to do this we'll need your help. Can you propose a fix as a pull request?

This is now a regression in a patch-level release, it broke all our headless testing.

I see the added milestone 3.0.0 and it alarms me.
Could you please clarify me your position on supporting non-browser environments?
The support is still in README and in intro.js.

It shouldn't have been landed in 2.1.2. I'll investigate.

@denis-sokolov could you write how exactly you are requiring jQuery in your code?

@denis-sokolov it had been assigned to 3.0 and only a small number of things were pulled back for this 2.1.2 release. We expect 3.0 to be out in a month or two so it's not going to be a long wait. We also need to find a way to test this to prevent regressions, which is the root cause of the problem in the first place.

Run jQuery in Node, it's simple:

var jqueryFactory = require('jquery');
var doc = jsdom.jsdom(html);
var w = doc.parentWindow;
var $ = jqueryFactory(w);

The best way to be sure this lands is to create a pull request. We do want it to happen.

@mzgol has stated that you won't release a new patch quite quickly. I understand the release process is complex in a case of a big project like jQuery and I make no judgement on your decision.

I want to make sure that you have the full information, however.
The way I see it, currently jQuery is completely broken on Node.js and everyone needs to blacklist version 2.1.2 in their package.json to fix their build. It's inconvenient.

everyone needs to blacklist version 2.1.2 in their package.json

Sorry, I assumed for a while that it's something that we broke in 2.1.1 and just didn't fix in 2.1.2 but now I see it was working correctly in 2.1.1: http://code.jquery.com/jquery-2.1.1.js. It was working a little by accident as in Node with jsdom the location.href check was throwing but there was a try-catch around it that was fixing it. This makes the problem more serious than I thought.

@denis-sokolov can you confirm that changing just this one line fixes the problem?

This is the kind of thing i fear would happen, i think we should start running smoke tests on environments like this, i.e. jsdom, phantom, etc;

Running the Promises/A+ tests in Node (see e1303a3) will fix this for Node but that's still the future.

@mzgol, yes, after I changed that line, ran grunt, and copied dist/jquery.js to my project, all my tests passed. I confirmed with this patch on top of master and on top of 2.1.2.

Yeah, although this is not an original intentent of it and does not cover other environments

Agree that we need smoke tests for these environments, that the Promises/A+ tests will help there for node, and that we need to enable JSHint options to disable all but window and document if possible so we don't accidentally use them. We're planning to have 3.0 out in January and it can address at least some of these.

@denis-sokolov there's a lot of work involved in pushing a release including sending it to the CDN providers so I would prefer not to do that all over again. It was a 24-hour process to get here because of various issues with the process, not all of which are fixed and some of which require manual steps.

I don't think we have a lot of node users at the moment and you can set your dependency specifically to 2.1.1 for now. Sorry. How exactly are you using jQuery on the node side?

@dmethvin, I run automated tests and automated measurements on code that in production is run in browsers. jsdom provides a much more convenient and fast environment than Phantom.js.

I also am preparing to use my environment-agnostic modules to analyze HTML documents in a pure server-side application.

OK; PR #1949 should serve as a basic smoke-test for non-browser environments; it still requires work, though.

For reference, this was fixed for 2.1-stable in 685fae9; smoke-testing added in a followup 998556f.