Shim
Closed this issue · 17 comments
This task makes quick work of updating paths, but what about configuring shims based on bower dependencies? For example, the bower.json for jquery-ui lists jquery as a dependency, but jquery ui doesn't require('jquery') anywhere, much like the vast majority of jquery plugins. It would be really convenient if shims could be setup like so:
requirejs.config({
...
shim: {
'jquery-ui': ['jquery']
}
});The simple solution would be to always create shims for any bower package with dependencies that is not already shimmed. A more advanced solution would involve scanning an AST to see if the module ever requires it's dependencies, indicating that a shim isn't needed.
Seems like it would be smart to make this optional based on a task config flag.
That's a cool idea. It's a little tricky to implement because we can often figure out a path, even if a dependency doesn't have a bower.json, but there's no way we could figure out a shim without it. Also, not every dependency needs to be shimmed. I guess it doesn't hurt to create shims, but it would bloat the file if we shimmed everything. So that leaves the AST, as you mentioned.
One more issue, shims sometimes need to be in a particular order. For instance, bootstrap's popover plugin relies on bootstrap-tooltip and jquery and you have to make sure to list them in the right order. It's been a little while since I messed around with shims so maybe I'm mistaken, but if I recall, the order is important and that could be challenging to sort out.
I didn't know that shimming was order-dependent, as it's done with an object literal, not an array. That would make me sad, because it would mean that RequireJS is depending on something that's not guaranteed.
@shannonmoeller I may have been wrong on that point. Need to double check :)
@shannonmoeller I'm referring to the deps array inside of a shim. Which is in guaranteed order.
shim: {
'backbone': {
//These script dependencies should be loaded before loading
//backbone.js
deps: ['underscore', 'jquery'], // <-- this guy
//Once loaded, use the global 'Backbone' as the
//module value.
exports: 'Backbone'
},
@robdodson Oh, sure. That makes more sense.
I think I'm going to close this because it feels outside of the scope of what we should be doing. If you need a shim you probably want to configure that manually. It would greatly expand the complexity of the task to try to infer which libraries need to be shimmed and which do not.
I was thinking about adding it as an option to grunt-bower-requirejs, but the shim section in generator-webapp is ~40 lines long. I don't think people want to stuff all of that into their gruntfile. I'll be happy to reopen the issue and continue the discussion if anyone thinks of a good alternative.
When this bower plugin automatically generated the paths in require.config method, why should I manually go through all the installed packages, discover the dependencies and set the shim dependencies for each library manually? I think implementation of this feature can help a lot ...
At least it can walk through the bower.json files and dump the dependencies into the config file into a commented out section.
Anyway thanks for cool bower extension!
Reopening because a lot of folks ask about it. Will try to find some time to work on this.
I would be eventually interested to work on this issue when time permits.
To determine the deps array in the correct order for each dependency, one can use the http://en.wikipedia.org/wiki/Topological_sorting algorithm on the dependencyGraph.
The question is how to specify the dependencies for which we will generate a shim : via a cli option or in the bower.json in the same way we did in #91 ?
That's excellent!
But I don't think topological sorting is necessary. You can just list
dependencies for package, then do the same recursively for every dependency
and so on until there'are no dependencies (i.e. leaf).
On 18 August 2014 11:56, Florian Voutzinos notifications@github.com wrote:
I would be eventually interested to work on this issue when time permits.
To determine the deps array in the correct order for each dependency, one
can use the http://en.wikipedia.org/wiki/Topological_sorting algorithm on
the dependencyGraph.The question is how to specify the dependency for which we will generate a
shim for : via a cli argument or in the bower.json in the same way we did
in #91 #91 ?—
Reply to this email directly or view it on GitHub
#64 (comment)
.
I think adding a cli option --shim backbone is the best way to specify the dependencies for which a deps array must be generated as mentioned in #64 (comment). The user will edit the config manually to add the exports property and so on if needed.
Does it cover your use case @shannonmoeller @kfrajtak ?
I would generate it for all libs :)
On 18 August 2014 15:08, Florian Voutzinos notifications@github.com wrote:
I think adding a cli option --shim backbone is the best way to specify
the dependencies for which a deps array must be generated as mentioned in #64
(comment)
#64 (comment).
The user will edit the config manually to add the exports property and so
on if needed.Does it cover your use case @shannonmoeller
https://github.com/shannonmoeller @kfrajtak
https://github.com/kfrajtak ?—
Reply to this email directly or view it on GitHub
#64 (comment)
.
Don't think that manually configuring the shims is a suitable option. What's the great benefit of configuring the shims in the gruntfile over configuring the shims inside the requirejs config?
It may be a good starting point but we should optimize it further to detect the complete shim automatically. I think about something like:
- parse AST and detect if a shim is necessary (e.g. look for AMD definition)
- for AMD Modules one should check if the module needs to be registered to the global namespace (had such issues with hammer.js plugins yet where the plugins required the global definition and the module only registered itself via amd)
- detect dependencies (calls to non declared methods, objects, ...)
- check if there's a suitable module for the dependencies installed
- determine deps array (e.g. like proposed in #64 (comment))
- look for the main definition for exports config (e.g. jquery.fn.module, window.module, ...)
- maybe it's even applicable to use something like https://github.com/SBoudrias/Inquirer.js for values that can not be resolved.
just thoughts.
I would generate it for all libs :)
We could add an other option like --shimAll.
- parse AST and detect if a shim is necessary (e.g. look for AMD definition)
IMO, it sounds like an unreasonable amount of work for this "simple" feature.
I think we need to need to provide "complete" shims. It's more complicated to search for partial shims in the requirejs config than adding the shims by yourself. Therefore we should take the "unreasonable amount of work" or drop the feature.
IMO, partial solutions will not provide the desired outcome to the user. It just complicates things
I tend to use bower as a 'client-side composer'. From this perspective, it already feels inconvenient enough to have to set up and run a grunt task to wire up all my app's dependencies, let alone figure out myself afterwards which ones require which other ones. I have better things to do with my time. In my ideal world, I would use $ bower install <package>, $ bower update and have these commands also automatically do what grunt-bower-requirejs does, including the shim (just as composer does). It should be up to package developers to keep their bower.json files clean. Unfortunately, AMD in the world of client-side JavaScript is not as standard as PSR autoloading is for PHP. Having grunt-bower-requirejs add full shims is a step in the right direction. Maybe some logic for resolving dependencies could be 'stolen' from composer?
👍 automatic shim creation for deps is very essential as it will help browsers to load scripts in proper order. I ended up adding shim/deps whenever I see script loading errors and that fixes the issue.