Bug Tracker

Ticket #13768 (closed bug: fixed)

Opened 20 months ago

Last modified 14 months ago

Error trying to load jQuery from node.js

Reported by: gleb.bahmutov@… Owned by: timmywil
Priority: high Milestone: 2.1
Component: build Version: 2.0.0-beta3
Keywords: Cc:
Blocking: Blocked by:

Description

Trying to load jquery from a small node.js script

var $ = require('./jquery');

Using latest build (e80117adb49eff8f82140b01d755bdeefe7eb089) and building using grunt gives:

L:\tests\node-test\jquery.js:8764
})( window );
    ^
ReferenceError: window is not defined
    at Object.<anonymous> (L:\tests\node-test\jquery.js:8764:5)
    at Module._compile (module.js:456:26)

Using official  2.0.0-beta3 gives slightly different error (because the top level object passed into the IIFE was this, not window)

L:\tests\node-test\jquery-2.0.0-beta3.js:35
        docElem = document.documentElement,
                          ^
TypeError: Cannot read property 'documentElement' of undefined
    at L:\tests\node-test\jquery-2.0.0-beta3.js:35:20
    at Object.<anonymous> (L:\tests\node-test\jquery-2.0.0-beta3.js:8764:3)

Change History

comment:1 Changed 20 months ago by rwaldron

  • Owner set to rwaldron
  • Status changed from new to assigned

Yep, that sounds about right...

http://bugs.jquery.com/ticket/13760

comment:2 Changed 20 months ago by timmywil

This is what we expected. jQuery is not meant to be loaded with Node alone as it doesn't work without the DOM. It needs something like jsdom to export a window object. That being said, we could provide limited support by doing window || this in the outro, but we couldn't guarantee it's functionality.

comment:3 Changed 20 months ago by dmethvin

I doubt it would get very far even if window was defined as this. We're not trying to make jQuery work standalone in the Node environment, just to make it more convenient to use as a module with things like jsdom. I'll explain that a bit better in the next blog announcement.

Just a thought ... what if we defined the exports like this so window didn't have to be global? Makes the invocation more error prone but would emphasize the dependency.

module.exports = function( w ){

   // If user passed in a window object, use it;
   // otherwise keep using the IIFE window variable
   if ( w ) {
      window = w;
   }
   ...
}
...
// User should do the require like this, passing in a
// suitably faked but functional window object.
var $ = require( "jquery" )( window );

comment:4 Changed 20 months ago by gleb.bahmutov@…

I agree that having window in general, maybe better error message?

In my case I wanted to use jQuery deferred from node.

comment:5 Changed 20 months ago by dmethvin

@gleb, in that case you want this:  https://github.com/jaubourg/jquery-deferred-for-node

comment:6 Changed 20 months ago by timmywil

  • Priority changed from undecided to high
  • Component changed from unfiled to build

Good idea Dave. I'd prefer to pass the right window to the IIFE. Maybe something like this:

 https://github.com/timmywil/jquery/compare/13768

comment:7 Changed 20 months ago by rwaldron

@timmywill Seems like a reasonable solution*, but I still want Domenic to chime in.

comment:8 Changed 20 months ago by m_gol

Passing window || this instead of window to IIFE would still break in the very beginning, on location = window.location in core.js:13 so this solves nothing really.

It's probably better to wait for Domenic on this instead of second-guessing how would people actually use it, though I like the last proposed idea.

comment:9 Changed 19 months ago by timmywil

I've emailed Domenic.

comment:10 Changed 19 months ago by timmywil

  • Owner changed from rwaldron to timmywil

I'll go ahead and take this one and follow up later.

comment:11 Changed 19 months ago by domenic@…

I think jQuery should stay as it is in 2.0. In particular, I think the proposed var $ = require( "jquery" )( window ) idea is bad.

The use case that's most important, in my opinion, is people using browserify or one of the many other CommonJS-in-the-browser systems. For those people, they just want var $ = require("jquery") to work; they don't want to pass in window, or even let defaults take over and do var $ = require("jquery")().

Another consideration is that jQuery should present a uniform interface to all module systems. AMD and the global-object "module system" both present jQuery as the $ function. Presumably when ES6 modules start coming to browsers, jQuery will do the same for those. CommonJS should be no different.

Note that in jsdom, jQuery is always run within the context of a global environment that has a window anyway, so that use case also wants the usual jQuery behavior. The proposed var $ = require("jquery")(window) would be a nice convenient syntax in some cases, but would break all cases that try to use jQuery as it exists today, as something that relies on window as an ambient variable.

---

Perhaps the best of both worlds, *if* this is an important use case, would be

// works in browserify etc., throws in Node.js
// unless of course you do `global.window = jsdom.createWindow()`.
var $ = require("jquery");

// for convenient jsdom/Node.js usage:
var window = jsdom.createWindow();
var $ = require("jquery").fromWindow(window);

But again I don't think this is terribly important; given the focus on byte-saving around here, I can't imagine it would pass muster.

comment:12 Changed 19 months ago by timmywil

@domenic: Sounds reasonable. One minor note.

We probably couldn't provide something like .fromWindow unless we did an early return where no window was available (in which case require("jquery") would not return anything except some object containing .fromWindow. That's already pretty ugly as the export would again be inconsistent. Judging from your comment as a whole, it seems the best solution is to leave 2.0 the way it is and suggest defining the window globally before requiring jQuery.

comment:13 Changed 19 months ago by domenic@…

@timmywil yeah, I wasn't sure how much initialization require("jquery") would do. In theory since $ is just a function, it could be no initialization, right? So window wouldn't actually be used until you called the function, e.g. $(".whatever") or $("<div>"). But I imagine in reality things are more complicated.

comment:14 Changed 19 months ago by m_gol

@domenic We do some support tests at the time of loading. Also, there's some initial caching going on (like location = window.location) amongst other things so, yeah, it's not so simple.

comment:15 Changed 19 months ago by timmywil

  • Status changed from assigned to closed
  • Resolution set to wontfix

Closing wontfix, but we should document this somewhere. Opened api issue  https://github.com/jquery/api.jquery.com/issues/296

comment:16 Changed 18 months ago by timmywil

  • Status changed from closed to reopened
  • Resolution wontfix deleted

I think we should discuss this further. Julian proposed having a custom build for NPM.

comment:17 Changed 18 months ago by timmywil

  • Status changed from reopened to assigned

comment:18 Changed 17 months ago by timmywil

@domenic: Knowing that jQuery needs a window at initialization time, is this looking any better to you?

 https://github.com/timmywil/jquery/compare/13768

Bottom line is jQuery can't be included (not just used) without a window, so making that explicit in the require call makes sense to me.

comment:19 Changed 17 months ago by Timmy Willison

  • Status changed from assigned to closed
  • Resolution set to fixed

Support CommonJS environments by accentuating the need for a window with a document. Fixes #13768.

Changeset: 1f67d07c60c37e60052db37fc03d42af482c2d03

comment:20 Changed 14 months ago by dmethvin

  • Milestone changed from None to 2.1
Note: See TracTickets for help on using tickets.