lazd/gulp-handlebars

Don't override other plugins file.defineModuleOptions

lfbittencourt opened this issue · 6 comments

I will try to be short and describe an issue I've faced here...

I precompile my Handlebars templates as AMD modules. In this templates, I have calls to custom helpers I've created. As I keep these helpers as AMD modules too, it would be nice parse that templates, find any custom helper call and inject them as dependencies to the precompiled module. Phew!

Why can't I do that? Because any file.defineModuleOptions.require I have is overwritten in https://github.com/lazd/gulp-handlebars/blob/master/index.js#L52.

Here you can see gulp-define-module author showing a way we can be sure we don't override other plugins. Does it make sense for you?

Thanks!

lazd commented

@lfbittencourt Thanks for the clear and thorough issue report. I totally understand what you're saying, I'll get a fix in for this soon.

lazd commented

@lfbittencourt can you try my fix for this issue by running these commands and report back?

npm remove gulp-handlebars
npm install git://github.com/lazd/gulp-handlebars.git#issue/60

Then, try your build and see if the module definition happens as expected. I wasn't sure what to do with the context and wrapper properties, but I made sure I didn't override any object on file.defineModuleOptions.

Thanks! I'll come back here as soon as I have tested it :-)

@lazd It's working smoothly now! About context and wrapper, I'd do exactly what you did.

Looking forward to the merge. A major version to fix #61 as well, maybe?

Thanks again for the quick fix!

lazd commented

Sounds like a plan, thanks for testing @lfbittencourt!

@lazd I see there's some discussion about #61, so this issue could be released as a minor version. There's no relation between the issues and it would be very nice release the fix ASAP (at least to me :-)).