Bug Tracker

Modify

Ticket #7102 (closed enhancement: fixed)

Opened 13 months ago

Last modified 6 weeks ago

Register jQuery as a CommonjS async module

Reported by: jrburke Owned by: jrburke
Priority: high Milestone: 1.7
Component: core Version: 1.4.4
Keywords: 1.7-discuss Cc:
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

There are a couple browser-based CommonJS-friendly loaders (RequireJS, Yabble) that can load modules that use an  async module format.

Ideally jQuery would register itself as a module using this format. This would allow jQuery to participate in browser module loading with these compatible loaders. Right now a patched version of jQuery is needed to accomplish this behavior. This approach also makes it easier for script loaders to use the new jQuery.readyWait feature to indicate other async scripts are loading and to not fire the jQuery ready callbacks until the scripts finish loading.

There is a test for this change here:  http://github.com/jrburke/requirejs/blob/master/build/jquery/test.html

and I verified the change works by the jquery-master.js patched jQuery file that is a sibling to test.html. I am happy to reconfirm the change if it is committed to the master repo.

I will attach a pull request link to this ticket that has the code change after I create the ticket so I can put the ticket number in the pull request.

Ticket #5742 can be closed out, superseded by this ticket.

Change History

comment:1 Changed 13 months ago by jrburke

comment:2 Changed 13 months ago by SlexAxton

I just want to say that it would be awesome to not have to patch jQuery to give it support for these async loaders. It kills my ability to use it on a cdn. It's almost no code... :D :D :D

comment:3 Changed 13 months ago by snover

  • need changed from Review to Commit
  • Priority changed from undecided to high
  • Component changed from unfiled to core
  • Milestone changed from 1.4.3 to 1.5

I put this on the list for jQuery 1.next. Since it has a pull request ready already it is possible it might still land for 1.4.3.

comment:4 Changed 13 months ago by snover

  • Status changed from new to open

comment:5 Changed 13 months ago by anonymous

I'd love to see this in 1.4.3 if possible. Looks like a simple change since there's a patch.

comment:6 Changed 13 months ago by jrburke

There has been new agreement in the AMD proposal to use "define" instead of "require.def", I plan on updating the patch tonight to reflect the change, upside is the patch will be slightly smaller.

comment:7 Changed 13 months ago by anonymous

The pull request has been updated to use define() instead of require.def() now in accordance with the Async Module API.

 http://github.com/jquery/jquery/pull/25

The specific update was done in this commit:  http://github.com/jrburke/jquery/commit/9223483c6631c046e09e31a754660c23e9479e01

and confirmed to work with RequireJS 0.14.3, which conforms to the async module API.

comment:8 Changed 13 months ago by snover

  • Milestone changed from 1.4.4 to 1.5

Retarget all enhancements/features to next major version.

comment:9 Changed 11 months ago by jitter

  • Owner set to jrburke
  • Status changed from open to assigned
  • Version changed from 1.4.2 to 1.4.4

Pull request available see comment 7

comment:10 Changed 11 months ago by jrburke

I did a small update to the pull request, registering jQuery as a named module. I used "jquery" instead of "jQuery" since the module IDs in CommonJS are normally mapped to file names at some point, and normally jQuery is distributed via file names that are in lowercase.

I made the change so that jQuery is more robust to mixing in environments where anonymous modules are supported but used alongside library code on the page that may have combined jQuery in with other scripts or modules using optimization strategies that are not aware of anonymous modules. When combining multiple anonymous modules together in a file, the combination process needs to be sure to give each anonymously define()'d module a name.

However, there will be many people using jQuery that use script combination processes that are not aware of anonymous modules, so it is best to name jQuery's define()'d module. This will allow it to work well in pages that mix anonymous-unaware code with other anonymous-aware code.

This is the same approach I recommend for any jQuery plugins that want to optionally opt-in to define()ing a module, and something I advocated recently at the online jQuery summit conference.

comment:11 Changed 10 months ago by jrburke

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

Register as a CommonJS async module if in that kind of environment. Fixes #7102.

Changeset: 6ffa730721a8ebcd128f3dc202706e46d9cfe249

comment:12 Changed 10 months ago by jrburke

Register as a CommonJS async module if in that kind of environment. Fixes #7102.

Changeset: 6ffa730721a8ebcd128f3dc202706e46d9cfe249

comment:13 Changed 10 months ago by wycats

Revert "Register as a CommonJS async module if in that kind of environment. Fixes #7102."

This reverts commit 6ffa730721a8ebcd128f3dc202706e46d9cfe249.

Conflicts:

src/core.js

Changeset: ae6655bcb6d852b79df2ddc5545832c5d08936f4

comment:14 Changed 10 months ago by jrburke

I will send another pull request that includes updated checking for define.amd and define.amd.multiversion, and it will include the test that csnover added as part of the original commit.

The AMD API page has been updated to contain information about the  define.amd property.

RequireJS has  added this property, and there is a  test page that confirms the jQuery change works with an AMD loader.

Please let me know if you need more information or changes. I will post back to this ticket with the pull request URL after publishing this comment.

comment:15 Changed 10 months ago by jrburke

comment:16 Changed 10 months ago by jitter

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone changed from 1.5 to 1.6

comment:17 Changed 10 months ago by jitter

  • Status changed from reopened to open

comment:18 Changed 7 months ago by john

  • Milestone changed from 1.6 to 1.next

Let's discuss this for 1.7.

comment:19 Changed 7 months ago by jrburke

I updated the pull request to this one (supersedes the old one):

 https://github.com/jquery/jquery/pull/331

It has *just* the changes needed for this ticket, and specifies a special define.amd.jQuery capability flag that is used to indicate if the AMD loader can deal with multiple versions of jQuery calling define. See the pull request for more info, and matching tests/changes in RequireJS to work with capability flag.

comment:20 Changed 5 months ago by john

  • Keywords 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:21 Changed 5 months ago by rwaldron

  • Description modified (diff)

+1, Alot of work has gone into this,  https://github.com/jquery/jquery/pull/331

comment:22 Changed 5 months ago by jaubourg

+0, I know why people want this but I'd rather wait for a standard module system in JS. Is it that difficult to embed jQuery as a module? Do we absolutely need this in core?

comment:23 Changed 5 months ago by ajpiano

  • Description modified (diff)

+1, Yes, we have to stop making this difficult for people.

comment:24 Changed 5 months ago by timmywil

  • Description modified (diff)

+1, I would love this.

comment:25 Changed 5 months ago by dmethvin

  • Description modified (diff)

+1

comment:26 Changed 5 months ago by john

  • Description modified (diff)

+1, Seems trivial enough!

comment:27 Changed 5 months ago by addyosmani

+1

comment:28 Changed 5 months ago by jzaefferer

+1, surprisingly little change necessary

comment:29 Changed 4 months ago by ajpiano

  • Description modified (diff)
  • Milestone changed from 1.next to 1.7

comment:30 Changed 7 weeks ago by thomasalwyndavis@…

Definitely a step forward for clearing up the global namespace.

comment:31 Changed 6 weeks ago by jrburke

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

Landing pull request 331. Add support for registering jQuery as an AMD module. Fixes #7102.

More Details:

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.