1.0rc1 breaks with use in RequireJS optimizer #333

Closed
andrehaveman opened this Issue Oct 10, 2012 · 11 comments

Comments

Projects
None yet
10 participants

I've written a RequireJS plugin that precompiles Handlebars templates during optimization. I use a shim config to load Handlebars with RequireJS. Since the rc1 release this doesn't work anymore. Beta6 had no problems.

I think it has something to do with that 'this' is not the same as the global scope in the RequireJS optimizer.

This is the code that causes the problem:

// lib/handlebars/base.js

/*jshint eqnull:true*/
this.Handlebars = {};

(function(Handlebars) {
    ...
}(this.Handlebars));

Later on it tries to add properties to a 'global' Handlebars variable, which causes a reference error.

Changing it back to the following solves the problem:

var Handlebars = {};

(function(Handlebars) {
    ...
}(Handlebars));

thoom commented Oct 12, 2012

This is the same issue I'm having with a server-side implementation I've written for node.

mole440 commented Oct 26, 2012

+1

Contributor

mpetrovich commented Jan 5, 2013

This may be a similar issue to one that happens with Uglify (a JS minifier/obfuscator). Our solution has been to wrap the whole shebang in an IIFE:

;(function(window, undefined) {
    /* everything in handlebars.js */
})(window);

giannif commented Jan 25, 2013

The scoping is messed up, at the beginning of the Handlebars.js it creates the object on this.

this.Handlebars = {};

Then later it's referenced just as Handlebars.

Handlebars.Parser = handlebars;

This throws an error unless Handlebars is in the global scope, because that's where it's looking.

Example of it failing:

var myRef = window.Handlebars;
window.Handlebars = undefined;
console.log(myRef.compile("{{test}}")({test:"doesn't work"}));  
// throws error at Handlebars.Parser = handlebars;

Try it here.

@ghost ghost assigned kpdecker Feb 16, 2013

Collaborator

kpdecker commented Feb 16, 2013

The entire content is now wrapped in a iife and the global object is defined with a var statement. This didn't make it into the rc3 release but should be available in the next.

@kpdecker kpdecker closed this Feb 16, 2013

This change broke RequireJS optimization (didn't tried non-optimized RequireJS usage). For Handlebars to work with RequireJS it has to be shimmed as it doesn't currently supports AMD (please consider adding UMD). RequireJS has a special feature to shim non-AMD modules based on which global they export. For example, this is the shim I use:

shim: {
  'handlebars': {
    exports: 'Handlebars'
  }
}

And RequireJS takes the global variable, removes it from global scope and makes a AMD module with it. Using Handlebars 1.0.0-rc.3 works fine, but since 1.0.10 it stopped working. I get undefined when getting Handlebars with RequireJS as if it couldn't find the global. Can we workaround on this?
I had to revert to 1.0.0-rc.3 for now.

+1 with this, "compile" is now undefined...

RaNaN commented Jun 2, 2013

I can also confirm it's broken in rc.4 and 1.0.0, please reopen this issue.

Collaborator

kpdecker commented Jun 3, 2013

@RaNaN http://builds.handlebarsjs.com.s3.amazonaws.com/handlebars-latest.js has the new loadering module that we are attempting for the 1.1 branch.

Can you give this a try and let me know if this works under you build environment? If it does not I'll gladly look into this if someone can provide me with a test project that I can use to verify any changes against (and ideally integrate into our testing environment :) )

RaNaN commented Jun 3, 2013

Thank you, everything works for me now :)

@kpdecker You totally just saved me a night of banging my head against the wall! I spent the last couple hours researching workarounds and various plugins to fix this issue, most of them didn't work and the rest didn't work for my specific situation. After using your handlebars-latest script it took me about 10-20 minutes of undoing all the extra layers I've added to get back to the basic installation for it to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment