-
Notifications
You must be signed in to change notification settings - Fork 11
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
Upgrading from 0.1.14 -> 0.2.4, global
should be defined
#15
Comments
I think it's related to the change in https://github.com/requirejs/cajon/blob/master/tools/c.js#L41. Using When |
If you have an example or test case that shows what kind of module loading breaks, that would help figure out a solution. |
Here's a CommonJS module to test global. https://gist.github.com/GCheung55/5116931959a24afc1b2a I just tried [email protected] and the issue is reproducible. |
Looking more at this, while the behavior changed in #8, it was for correctness -- best not to leak the private variables inside the loader to the evaluated text, and to provide a consistent eval environment. So I would expect the behavior as Is this more about compatibility with modules from node? If so, that is a trickier issue. Node has other implied available variables, like If this was needed, for a particular project, it can be provided outside the loader: <script src="cajon.js"></script>
<script>
var global = this;
require(['app']);
</script> Does that meet the goals? |
True, I could add How do you feel about exposing global as an argument to the module's callback? define(function(require, module, exports, global){}) I ask because this would be a tiny bit more performant than looking up the scope chain in order to find |
Also, there are node modules that are written for both browser and server environments. The one I'm currently using is https://github.com/kamicane/prime/blob/master/defer.js which makes use of checking |
Upgrading from 0.1.14 -> 0.2.4,
global
should be defined but an JS error is being thrown that "global is not defined".The text was updated successfully, but these errors were encountered: