#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: |
Description
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 8 years ago by
comment:2 Changed 8 years ago by
Component: | General → Query |
---|---|
Owner: | set to Kris Zyp |
comment:3 Changed 8 years ago by
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 8 years ago by
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 7 years ago by
Type: | defect → enhancement |
---|
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 5 years ago by
Milestone: | tbd → 1.12 |
---|---|
Resolution: | → patchwelcome |
Status: | new → closed |
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 5 years ago by
I opened a pull request to address this issue https://github.com/dojo/dojo/pull/211
comment:8 Changed 5 years ago by
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].
Please set component to "Query". I forgot to set it on creation and it seems I can't set it now.