Opened 9 years ago

Closed 6 years ago

Last modified 6 years ago

#16393 closed enhancement (patchwelcome)

Unnecessary use of custom plugins in query support

Reported by: Randy Hudson Owned by: Kris Zyp
Priority: undecided Milestone: 1.13
Component: Query Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:


It would be really helpful if Dojo could minimize its use of custom AMD plugins. Specifically, the way dojo/query uses custom plugins:

  • makes consumption of dojo more difficult in AMD environments which only support server-side plugins
  • may cause poor runtime performance due to unnecessary XHR to fetch the implementation
  • increases javascript code in the browser (minor)

Our AMD support has an extremely small client-side footprint ( < 100 lines), which is only possible because all use of AMD plugins is processed on the server. We don't support plugins which can't be optimized during the "build" phase.

Instead of query.js being a custom plugin as well as having a dependency on a second custom plugin: ./selector/_loader!default, query.js can simply be replaced with [a slightly modified] acme.js. There is no need to implement a custom plugin to do something already directly supported in AMD.

If someone wants to configure the default/global query engine to be the "lite" engine, the AMD mechanism for this is using a config entry to map the id "dojo/query" to the path "dojo/selector/lite".

If a specific module wants to consume the "lite" engine directly, it can do so using straight AMD:

define(["dojo/selector/lite"], function(query){
  query(".someClass:custom-pseudo").style("color", "red");

and NOT

define(["dojo/query!dojo/selector/lite"], function(query){
  query(".someClass:custom-pseudo").style("color", "red");

I realize that for backwards compatibility, dojo <2.0 needs to preserve pre-AMD mechanisms like selectorEngine: 'xyz' set via the dojo config, but the long-term strategy is AMD. It would be a shame to see the 2.0 AMD story become unnecessarily complicated just because of the way this transition period was handled.

As a workaround, we have modified our copy of dojo/query so that the dependency on "./selector/_loader!default" is now just "./selector/acme".

Change History (8)

comment:1 Changed 9 years ago by Randy Hudson

Please set component to "Query". I forgot to set it on creation and it seems I can't set it now.

comment:2 Changed 9 years ago by James Thomas

Component: GeneralQuery
Owner: set to Kris Zyp

comment:3 Changed 9 years ago by bill

The goal with the current design is to download (the large file) acme.js on old browsers like IE6, but to leverage querySelectorAll() on newer browsers, especially mobile devices.

comment:4 Changed 9 years ago by Kris Zyp

Is your concern more focused on Dojo 2.0? Based on the supported browsers, I think that dojo/query will be significantly simpler (from a module dependency perspective) in 2.0.

comment:5 Changed 8 years ago by cjolif

Type: defectenhancement

in any case sounds more like a enhancement than a bug? Nothing is broken, this is just suggesting a different way of proceeding?

comment:6 Changed 6 years ago by dylan

Milestone: tbd1.12
Resolution: patchwelcome
Status: newclosed

Given that no one has shown interest in creating a patch in the past 2+ years, I'm closing this as patchwelcome.

comment:7 Changed 6 years ago by jrowe

I opened a pull request to address this issue

comment:8 Changed 6 years ago by bill

That PR looks good to me but it's different than this ticket. Should open another ticket saying that dojo/query doesn't work with requirejs [at build time].

Note: See TracTickets for help on using tickets.