-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot define singletons #145
Comments
I'll think about this some more but I think your workaround (going to globals while the singleton is in both places) is a good solution. It's annoying but Sprockets hands us a stream of JavaScript and browserify is run on it. Apparently, the module identifiers are different for the different code paths when pulling in the singleton. I don't know if there is a way to have browserify use consistent identifiers -- ie based on the content instead of just assigning a numeric id. It suspect a solution would require overriding the identifier to be based on the file path. I believe it is technically possible but might be a fair amount of work to implement. |
Oh, and is the build time going up 5 minutes due to using browserify-rails? If so, are you using browserify-incremental? My recommendation would be use this gem to get to CommonJS and then jump off to browserify or webpack building directly. It'll give you better performance. |
Wait -- so I think this problem is basically |
See this example: https://github.com/substack/node-browserify#external-requires So might not be |
We're using the default settings, which I think uses browserify-incremental, doesn't it? At least, we're not doing anything to disable it... I'm going to have to educate myself a little on the internal vs external bundling. To the best of my understanding this is all within the same bundle, or does using sprockets requires end up causing multiple bundles to be built separately? |
You're ending up with one asset file however when you take multiple dives from sprockets JS -> CommonJS, you're ending up with separate "worlds" of modules. So when the singleton module is resolved in one "world", it's not the same as the other "worlds" (if the dives were separate). A world is really a JavaScript scope in the end. By using these other settings, you can tell browserify a module should be resolved outside of the "world" -- at a more global scope. So the sprocket requires do indeed cause something like multiple bundles but due to being in the asset pipeline, unless you explicitly configure another bundle, they'll all end up in the same file. |
Makes sense, thanks for the lesson. A lot of stuff is global in the current application already, so might stick with that approach for the time being and start scoping it down after we get rid of the sprockets stuff. We've been localizing most of the stuff as we go, but this throws a wrench in the process. |
Yeah, I can understand that. I put a warning in the README a while back but I didn't realize the pattern we're talking about would break singletons until you pointed it out. So now I have a more concrete reason why it is best to have one jump from Sprockets -> CommonJS. I'll keep thinking about this. It's a thorny issue. |
TL;DR Any summary on possible workaround? |
We ended up keeping things that needed this behavior on the global scope until we were able to remove all of the sprockets requires. |
@mockdeep Wish there is a way to use |
@PikachuEXE you can still use // application.js
require('my_stuff');
// my_stuff.js
window.my_stuff = window.my_stuff || { my: 'stuff' };
module.exports = window.my_stuff; Alternatively, you can do something like: // application.js
window.my_stuff = require('my_stuff');
// always refer to `window.my_stuff` in your code
// my_stuff.js
module.exports = { my: 'stuff' }; |
@mockdeep |
@PikachuEXE ah, yeah. We ran into that, too. We ended up assigning all of those to the global scope as well: // application.js
window.underscore = require('underscore');
// elsewhere.js
var _ = window.underscore; We did it this way for a couple of reasons, rather than assigning |
The best case we think: (if this issue doesn't exist) // application.js
//= require_tree ./some_component
// components/some_component.es6
const _ = require('underscore'); So now we should have something like this? // application.js
//= require_tree ./dependencies
//= require_tree ./some_component
// dependencies/underscore.js
window.underscore = require('underscore');
// components/some_component.es6
const _ = window.underscore;
// so stuff Just not sure how to specify the dependencies easily for different components |
That looks like it'll work so that your dependencies are loaded first. Otherwise, you can add them to One piece of advice I would give, though, is get rid of all // application.js
//= require ./dependencies/underscore
//= require ./components/some_component
// dependencies/underscore.js
window.underscore = require('underscore');
// components/some_component.es6
const _ = window.underscore;
// so stuff We actually wrote a script to automate some of the conversions and check them in a handful at a time. I'm not sure how helpful it'll be, since it's pretty specific to our setup, but I added a gist here. |
(Slightly updated snippet in comment, so you may want to take another look.) |
Came across this issue yesterday. We're transitioning our app gradually to
browserify
, so we've got places in our code that have sprockets requires and other places that we're doingbrowserify
requires. (No more places where we've got sprockets requires andmodule.exports
in the same file, though.) The issue seems to be that when we have a sprockets require of a file that itself has a node style require, the node style require gets re-run. A slightly editorialized example, so please forgive inconsistencies:Output:
We've been seeing a pretty massive performance impact due to our transition, ~5 minutes to build, and I'm thinking maybe this has something to do with it. We're requiring
underscore
in a handful of places, so if that is having to be re-evaluated each time then it could be pretty costly on its own. We'll probably end up having to assign things to globals for the time being to reduce the performance hit until we can get rid of all of our sprockets requires.The text was updated successfully, but these errors were encountered: