-
Notifications
You must be signed in to change notification settings - Fork 159
Shimming Backbone and Underscore for RequireJS unnecessary? #101
Comments
@raphaelvalerio I am not a requirejs expert. but surely get someone to comment on this and fix this. @addyosmani / @passy Please help to resolve this. |
@revathskumar @addyosmani @passy If the shims should indeed be removed, I can easily submit a pull request with the fix. Just confirm that they shouldn't be there, and I'll take care of it. |
I think it's better to leave the shims in place, and instead use the actual Backbone and underscore. |
I think if the generator makes it explicit (in the readme or the description) that the downloaded versions of Backbone and Underscore are the AMD compatible versions there is no need of the shims. Conversely, we can use the vanilla versions and use the shims only. My only concern is that if a new version comes out and somebody upgrades with a non-AMD version things can break if they are no aware of our choice. |
So I think we can keep the shim and use the non-AMD (official) backbone and underscore. If so we need to change only the bower.json. |
I've been using the non-AMD ones, and everything is working the same. |
Remember that the generator offers you a choice on the command line during scaffolding, where you can choose whether you want to install RequireJS for AMD support or not. This choice totally changes the way the scaffolded app ends up being created. If you DO choose to install RequireJS, here's what happens:
If you DO NOT choose to install RequireJS, here's what happens:
The point of all of this is that if you scaffold an app with You should not need to shim an AMD-ready JS file, and according to the RequireJS documentation, doing so can cause the potential for trouble. I've removed the shim declarations from the Think of the choice made at scaffold as implicitly creating a forked version of |
That is right. I just ran the generator and it does ask if I want Require.js support and then downloads the AMD version. I also tested it without the shims (I only needed to give the paths for Backbone and Underscore) and everything worked. |
Let me clarify: I am running the Requirejs scaffolded install with the official libraries (and the shims they need) and everything is working fine. The shimming system itself is robust, and as long as the libraries keep everything they need under a single object namespace, it's not going to break. I think it's better if we think of the scaffolding as |
@jridgewell I'm not sure I understand what you think we should be doing (although I do understand and definitely agree with what you mean about making a distinction between the version scaffolded with and without RequireJS as if they were almost separate generators). Do you think we should be leaving the shims in place in the Require version of the scaffolded app or do you think they should be removed? I'm guessing you're saying that we should leave them in, so I'll respond under that assumption (please correct me if I'm wrong) -- If we scaffold with RequireJS, the shims for Underscore and Backbone aren't necessary because the versions of Underscore and Backbone are AMD forks of those libraries and don't need to be shimmed. They work without the shims. They also seem to work with the shims, but here's the quote from the RequireJS docs that suggests that's not a good idea:
Both So from my point of view, there are two big reasons to remove the shims from the RequireJS version of this generator: 1) Because the RequireJS docs say shimming an AMD module is a bad idea, and 2) it's unnecessary and just wastes bytes. Unless there's a genuine reason why those shims must be there that I'm unaware of, I contend that we should remove them from the RequireJS version of |
You're correct, I think the shims should be left in, but only because we should switch away from |
@jridgewell Ah, now that's an interesting point. I kind of like that idea. I don't know anything about the people behind the To distill down this shims issue (unless the main authors of When the user chooses to install RequireJS during scaffolding, either:
I'm leaning toward option 1, like @jridgewell, because I do like the idea of sticking to the main Backbone and Underscore release paths. What do others think? |
Number 1. is the way to go. FWIW I've been using Backbone + Underscore non-AMD with the shim config for a while and there are no issues. |
Thanks everyone for the comments. @raphaelvalerio Can you please help to fix this with a PR? Thanks. |
Done! I'll close this issue thread so we can move the discussion over to the pull request if needed. |
Fixes #101 : Use (and shim) core Backbone and Underscore Bower packages for scaffolding with RequireJS
Hi All, I just wanted to ask if this needs updating as starting from version 1.1.1 "Backbone now registers itself for AMD (Require.js), Bower and Component, as well as being a CommonJS module and a regular (Java)Script. Whew." See http://backbonejs.org/#changelog. Same for Underscore starting from 1.6.0 "Underscore now registers itself for AMD (Require.js), Bower and Component, as well as being a CommonJS module and a regular (Java)Script. An ugliness, but perhaps a necessary one." See http://underscorejs.org/#changelog. So no need for shims? |
Exciting! Let's investigate that. I hate that shimming nonsense. |
Fixes #101 Backbone and Underscore are now properly AMDified, so we don't need to define a shim config for them anymore.
Hi everyone,
When you scaffold an app with generator-backbone and choose to use RequireJS, the resulting
main.js
file shims both Backbone and Underscore inrequire.config
.However, the Bower installs for both of these libraries are the AMD versions, and as far as I can tell, they both are fully AMD compatible out of the box. According to the RequireJS documentation, you should never shim a module that already has a
define()
call as it can cause trouble.On my test app, I've commented out the shim declarations and have no trouble with using Backbone or Underscore in my RequireJS-rigged app, so as far as I can tell the shims are unnecessary.
Am I missing something here? Is there a reason that you're shimming Underscore and Backbone that I'm too stupid to be aware of, or should the shim stuff be removed?
The text was updated successfully, but these errors were encountered: