Skip to content
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

excludeHbsParser does not seem to have any effect #244

Open
chearon opened this issue Oct 21, 2015 · 5 comments
Open

excludeHbsParser does not seem to have any effect #244

chearon opened this issue Oct 21, 2015 · 5 comments

Comments

@chearon
Copy link
Contributor

chearon commented Oct 21, 2015

I followed the example.build.js found in the demo directory, but setting all of the pragmas still leaves me with the full Handlebars compiler built into my source.

Grepping 1.0.0 for excludeHbsParser gives no results other than the build config, so I don't think it's actually being used anywhere.

That's the (I think) objective thing I found, now if I may make a couple suggestions:

  1. The readme could be amended to provide something like the build config to show people how to optimize their build. I suspect most users have the entire Handlebars compiler built into their project
  2. Could we just adopt the same config as the requirejs text plugin, and use stubModules for the optimized plugin? To do that you'd need to (a) not require hbs in compiled templates and (b) get it to load handlebars.runtime.js which the compiled templates need, maybe by having users adjust their paths.config
@chearon
Copy link
Contributor Author

chearon commented Oct 22, 2015

I was able to achieve the right build without having to fork the code:

   stubModules: ['hbs', 'hbs/underscore', 'hbs/json2'],
   onBuildWrite: function (moduleName, path, contents) {
     if (moduleName === 'hbs/handlebars') {
       return grunt.file.read('vendor/require-handlebars-plugin/hbs/handlebars.runtime.js')
         .replace('define(factory);', 'define(\'hbs/handlebars\', factory);');
     } else {
       return contents;
     }
   }

I still think the pragmas could be dropped and the documentation updated, I'm willing to help. Seems like it's been agreed that's the best way to do plugins according to @jrburke here requirejs/r.js#116

@SlexAxton
Copy link
Owner

Happy to take a PR for it.

On Thu, Oct 22, 2015 at 3:13 PM Caleb Hearon [email protected]
wrote:

I was able to achieve the right build without having to fork the code:

stubModules: ['hbs', 'hbs/underscore', 'hbs/json2'],
onBuildWrite: function (moduleName, path, contents) {
if (moduleName === 'hbs/handlebars') {
return grunt.file.read('vendor/require-handlebars-plugin/hbs/handlebars.runtime.js')
.replace('define(factory);', 'define('hbs/handlebars', factory);');
} else {
return contents;
}
}

I still think the pragmas could be dropped and the documentation updated,
I'm willing to help. Seems like it's been agreed that's the best way to do
plugins according to @jrburke https://github.com/jrburke here
requirejs/r.js#116 requirejs/r.js#116


Reply to this email directly or view it on GitHub
#244 (comment)
.

@chearon
Copy link
Contributor Author

chearon commented Oct 25, 2015

See what you think of my branch for this issue: https://github.com/chearon/require-handlebars-plugin/tree/issue-244

I didn't have to change hbs.js except to remove the pragmas. So I've just updated config/readme to show the stubModules way of optimizing out the plugin. The demo-build/main.js is now like 4,000 lines shorter because it stubs out the whole module this time

@SlexAxton
Copy link
Owner

Yea. Might need a couple more words in the readme (or a link) on how to get the optimized output, but the code looks great. PR me?

@chearon
Copy link
Contributor Author

chearon commented Oct 25, 2015

cool! Just added some stuff to the readme and opened a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants