Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#16561 closed defect (patchwelcome)

dojo/on uses a global (instead of AMD) reference to dojo.query

Reported by: Mangala Sadhu Sangeet Singh Khalsa Owned by: Kris Zyp
Priority: undecided Milestone: tbd
Component: Core Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

Line 180:

matchesTarget = matchesTarget && matchesTarget.matches ? matchesTarget : dojo.query;

Change History (15)

comment:1 Changed 7 years ago by bill

Component: GeneralCore
Owner: set to Kris Zyp
Status: newassigned

Kris put that there intentionally, so basic dojo/on can work without loading dojo/query, but it bothers me too, and obviously will have problem in 2.0 or even in 1.x when packages are remapped.

comment:2 Changed 7 years ago by ben hockey

it's actually not a global reference... it relies on dojo/query adding the query property to dojo/_base/kernel (aka dojo). so it works fine as part of AMD even with remapped packages. yes, it's tricky code but it's not "wrong" per se. it's definitely something that would be nice to improve if possible.

comment:3 Changed 7 years ago by ben hockey

Resolution: patchwelcome
Status: assignedclosed

i'm going to resolve this ticket with "patchwelcome" because these kinds of tickets tend to stick around forever without any action.

comment:4 Changed 7 years ago by Wouter Hager

Not that it really matters, but I don't understand how this works. When I use dojo uncompressed this line gives errors when the selector is a string (e.g. for dijit/Tree). Is that a problem?

comment:5 Changed 7 years ago by bill

We test uncompressed all the time so I'm not sure what you are seeing, or why we aren't seeing it. It sounds like a problem but you'd need to give a test case to reproduce it.

comment:6 Changed 7 years ago by Kris Zyp

Perhaps it is due to our tests including dojo/query somewhere else (from base?), and his tests are baseless. dijit/Tree really should be including dojo/query as a dependency if it is going to use event delegation/selectors (http://dojotoolkit.org/reference-guide/1.8/dojo/on.html#selector).

comment:7 Changed 7 years ago by Wouter Hager

Yes.

comment:8 Changed 7 years ago by Wouter Hager

No. When I create a dijit/Tree by the example on http://dojotoolkit.org/reference-guide/1.8/dijit/Tree.html, but instead use dojo uncompressed, the tree won't respond. All events throw an error TypeError?: matchesTarget is undefined. Even without async:true and dojo/query required.

comment:9 Changed 7 years ago by Wouter Hager

dojo/query is indeed global when uncompressed is used, and not available in kernel.

comment:10 Changed 7 years ago by bill

In [30515]:

add missing (indirect) dependency from dijit/Tree to dojo/query, needed since on.selector() used with a string for the selector, refs #16561 !strict

comment:11 Changed 7 years ago by Wouter Hager

This is not sufficient to fix uncompressed. Perhaps just to patch it:

if(!dojo.query) {
dojo.query = dojo.global.dojo.query;
}

comment:12 Changed 7 years ago by Wouter Hager

Hmm something is wrong here. In uncompressed global is not available. Perhaps this has to do with load order?

comment:13 Changed 7 years ago by Wouter Hager

Sorry to bother! I probably corrupted my code somewhere, and didn't have the correct kernel require. Apologies.

comment:14 Changed 7 years ago by Wouter Hager

As for dojo/query, it's in _Widget, so it's not required in any widget. Sorry again.

comment:15 Changed 5 years ago by rwadkins

I'm putting together a tiny build of dojo and didn't need query but am using event delegation. this tripped me up. Now I have to depend on query, too, to get it added to the global. If on uses dojo/query, then the dependency should be stated in on. dojo.query is created as a side effect of another action and shouldn't be relied on.

Note: See TracTickets for help on using tickets.