Removing define.amd.jQuery

295 views
Skip to first unread message

James Burke

unread,
Jan 31, 2013, 11:26:31 AM1/31/13
to amd-im...@googlegroups.com
I have done this pull request for jQuery:

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

because I thought most loaders were just setting that value anyway.
However, while I find the value set in requirejs, almond, curl ad
lsjs, I do not see it in dojo or inject. I believe this is because
those loaders are targeted towards more uniform, all AMD loading
environments, and would be fine with this change.

So, loader implementers, do you anticipate problems if this check in
the jQuery code is removed? It would be good to hear back from loader
implementers, hopefully by Monday to confirm the change is good to go
into both the jQuery 1.x and 2.x trees.

It means jQuery will call define with a named 'jquery' define just if
define and define.amd exist.

This extra define.amd.jQuery check was put in there originally because
it is not uncommon for a page to have two jQuerys loaded on a page. It
was unclear what kind of impact that would have on loaders,
particularly since it could just be in the page as a plain script tag,
not a tag added by the loader. More details here:

https://github.com/amdjs/amdjs-api/wiki/jQuery-and-AMD

However, I believe the impact on loaders is that the developer would
need to just work with the loader to do the right thing. Common
solutions:

* just place the unwanted jquery before the loader's script tag
* Call jQuery.noConflict(true) in an app's module so the other jQuery
is the publicly visible one.
* Get the desired jQuery for the AMD loader in first, because the
loader only accepts the first define call and silently ignores the
second one for the same module ID.

So, give a holler if you see a problem.

James

Jakob Heuser

unread,
Feb 1, 2013, 8:17:28 PM2/1/13
to amd-im...@googlegroups.com
One of the reasons Inject held back on the define.amd.jquery descriptor was that we didn't have a good answer for how to solve the following define call

define(['lib/jquery'], function(jq) { /* ... */ });

With the jquery object defined in a named manner, you'd end up with jq being "undefined" or worse, the factory never running (as the "lib/jquery" module never technically loaded). Do other AMD users just stick jquery at their module roots, making this a non issue?

--Jakob

James Burke

unread,
Feb 1, 2013, 9:09:31 PM2/1/13
to amd-im...@googlegroups.com
On Fri, Feb 1, 2013 at 5:17 PM, Jakob Heuser <ja...@felocity.com> wrote:
> With the jquery object defined in a named manner, you'd end up with jq being
> "undefined" or worse, the factory never running (as the "lib/jquery" module
> never technically loaded). Do other AMD users just stick jquery at their
> module roots, making this a non issue?

Yes, placing it in the baseUrl, or using paths config:
https://github.com/amdjs/amdjs-api/wiki/Common-Config#wiki-paths

to point to that file:

requirejs.config({
paths: {
'jquery': 'lib/jquery'
}
});

Keeping the dependency name as just 'jquery' is usually needed anyway
when using third party AMD modules: those other modules cannot assume
a directory layout in the final project, and so just use 'jquery' as
the dependency ID, so that all of those types of modules get the same
module value.

James

Richard Backhouse

unread,
Feb 3, 2013, 10:11:26 AM2/3/13
to amd-im...@googlegroups.com
James,

I don't see this being a problem for my AMD loaders. I never did
anything in them to enforce single jquery instances.

Richard

James Burke

unread,
Feb 5, 2013, 1:35:12 PM2/5/13
to amd-im...@googlegroups.com
Rawld, Jakob, any other concerns? It is fine if you want more time to
think about it, but I believe both of your OKs are needed before going
ahead with the pull request.

James

Jakob Heuser

unread,
Feb 5, 2013, 2:35:12 PM2/5/13
to amd-im...@googlegroups.com

After talking to the team, we are okay with the change. Since jQuery still writes to the window object, it will be backwards compatible with our current recipes.

as long as it is never
define("jquery", [], jQuery.noConflict())
We should be okay.

Thanks James for reaching out to everyone.

--
You received this message because you are subscribed to the Google Groups "amd-implement" group.
To unsubscribe from this group and stop receiving emails from it, send an email to amd-implemen...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


James Burke

unread,
Feb 5, 2013, 2:42:50 PM2/5/13
to amd-im...@googlegroups.com
On Tue, Feb 5, 2013 at 11:35 AM, Jakob Heuser <ja...@felocity.com> wrote:
> as long as it is never
> define("jquery", [], jQuery.noConflict())
> We should be okay.

Agreed. That change is not part of that pull request, just the removal
of the define.amd.jQuery check. I do not expect the jquery signature
to change to that type of signature, since it would break jquery
plugins that depend on the jQuery global, because many of them are not
AMD-aware, but still loadable by AMD loaders.

James

James Burke

unread,
Feb 12, 2013, 11:23:42 PM2/12/13
to amd-im...@googlegroups.com
I spoke with Rawld offlist, and he does not believe it should affect
dojo if it is there or not, so I will let the jQuery folks know in the
ticket it is cleared from our side.

James
Reply all
Reply to author
Forward
0 new messages