Create exports.js for exporting jQuery to window and AMD. #586

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

jrburke commented Nov 8, 2011

This is in reference to ticket 10687. It is an updated pull request that supersedes #584. This ticket is based on feedback from @timmywil who asked for a new file that could be included in the tests so that the amdDefine test could be preserved.

The output of this file is the same as in #584 which was tested with the curl and requirejs AMD loaders. I also tested the built file from this branch with requirejs just for good measure.

Member

mikesherov commented Nov 8, 2011

Should exports.js be wrapped in (function(jQuery){}(jQuery); like the other modules?

Owner

timmywil commented Nov 8, 2011

In this case it's not a big deal, but it wouldn't hurt.

Contributor

jrburke commented Nov 8, 2011

Since exports.js does not declare any local variables that should be hidden from the other jQuery pieces or from the global scope, then I can see a case for not adding the wrapper, to keep the size of jQuery in check.

However, if you want it in there, just post back a confirmation and I will add it.

Owner

timmywil commented Nov 8, 2011

The makefile should remove all closures from each individual module and leave only the intro/outro in the built file.

Contributor

jrburke commented Nov 8, 2011

Wow, I completely misread the Makefile, I scanned right over the dots in the regexp. I updated the patch to include the wrapper.

Owner

timmywil commented Nov 14, 2011

Landed in commit 8bc60ba.

timmywil closed this Nov 14, 2011

@mescoda mescoda pushed a commit to mescoda/jquery that referenced this pull request Nov 4, 2014

@jrburke jrburke + timmywil Landing pull request 586. Create exports.js for exporting jQuery to w…
…indow and AMD. Fixes #10687.

More Details:
 - jquery#586
 - http://bugs.jquery.com/ticket/10687
9351c9e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment