Bug Tracker

Modify

Ticket #13760 (closed bug: fixed)

Opened 13 months ago

Last modified 13 months ago

getComputedStyle no longer works in node with jsdom

Reported by: m_gol Owned by:
Priority: high Milestone: None
Component: build Version: 2.0.0-beta3
Keywords: Cc:
Blocking: Blocked by:

Description (last modified by m_gol) (diff)

I've noticed in this pull request:
 https://github.com/jquery/jquery/pull/1103
a change to the parameter passed to the main enclosing IIFE, i.e. window was changed to this. The problem is, there are lots of places in jQuery where we assume window is actually window so it breaks a lot. One example is in css.js:

// NOTE: we've included the "window" in window.getComputedStyle
// because jsdom on node.js will break without it.
function getStyles( elem ) {
	return window.getComputedStyle( elem, null );
}

So we have a few bytes (window.) that actually fix nothing. Another example is window.addEventListener etc.

Change History

comment:1 Changed 13 months ago by m_gol

I meant "a few bytes that fix nothing", not "save".

comment:2 Changed 13 months ago by rwaldron

What a nightmare. There are way too many places where we need window to be _window_

Last edited 13 months ago by rwaldron (previous) (diff)

comment:3 Changed 13 months ago by m_gol

Dave suggested we could just pass window instead of this and thus attach jQuery to window. jQuery is exported for node separately, anyway, so it shouldn't break.

If we need the global object inside, too, though, I see no other option than

(function ( global, window ) {
    /* code */
})( this, window );

comment:4 Changed 13 months ago by timmywil

  • Priority changed from undecided to high
  • Status changed from new to open
  • Component changed from unfiled to build

If I understand this correctly, jQuery won't work without jsdom and its window export, which means using node's global object isn't sufficient on its own. So, just passing window and continuing to assign to module.exports in the outro is probably the best option.

comment:5 Changed 13 months ago by m_gol

  • Description modified (diff)

comment:6 Changed 13 months ago by m_gol

It's actually not src/outro, it's in src/exports.

Pull request:  https://github.com/jquery/jquery/pull/1238

comment:7 Changed 13 months ago by Michał Gołębiowski

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

Fixes #13760. Pass window, not this to the main IIFE. Closes gh-1238

Changeset: e80117adb49eff8f82140b01d755bdeefe7eb089

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.