Skip to content
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

Preserving [global] as [this] when executing a yui module in nodejs. #1360

Merged
merged 4 commits into from
Nov 13, 2013

Conversation

caridy
Copy link
Member

@caridy caridy commented Oct 30, 2013

  • when executing a module that has some code other than of the YUI.add() statement, that code
    is suppose to run in a closure where this is equal to nodejs' global variable,
    which is the equivalent to window on the browser. That way we maintain feature
    parity for polyfills and other vendor scripts.

Here is an example:

(function (global) {
    // this is a very common pattern for polyfills
    global.bar = 1;
})(this);

YUI.add('foo', function (Y) {
     // Y.config.global.bar === 1;
}, '', {});

In nodejs, Y.config.global is global, while on the browser, it is window. The problem here is that this is not global when executing the polyfill code, but on the browser is it window. This PR is meant to solve this inconsistency.

Note: this change is very low risk because YUI developers are normally not dealing with this use-case, but in the near future they will see more and more of this when we start using more polyfills across the board.

TODO:

  • unit tests

 * when executing a module that has some code other than of the `YUI.add()`, that code
   is suppose to run in a closure where `this` is equal to nodejs' `global` variable,
   which is the equivalent to `window` on the browser. That way we maintain feature
   parity for polyfills and other vendor scripts.
@caridy
Copy link
Member Author

caridy commented Nov 13, 2013

need a final review on this. /cc @ekashida @ericf

// and optionally, adding the value of `foo` to the namespace
YUI.add('vendor-script-signed', function (Y) {
Y.foo = Y.config.global.foo;
}, '', {});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline.

@clarle
Copy link
Collaborator

clarle commented Nov 13, 2013

Looks good to me, and makes sense for both client-side and server-side YUI. 👍

@ericf
Copy link
Member

ericf commented Nov 13, 2013

Yeah this seems good and makes sense after we talked through what's actually going on here. Agreed that it's unlikely to affect anyone using YUI by the change of this going from exports to global when the code in the outer scope of their YUI module file is executed in Node.js.

@caridy caridy merged commit 7534825 into yui:dev-master Nov 13, 2013
caridy added a commit that referenced this pull request Nov 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants