Add support for passing options to the htmlbars precompiler #11
Conversation
| }); | ||
|
|
||
| assert.equal(transformed, "var compiled = Ember.HTMLBars.template(precompiled(hello));", "tagged template is replaced"); | ||
| }); |
There was a problem hiding this comment.
Looks like this test has been duplicated unintentionally?
| babel.transform(code, { | ||
| blacklist: ['strict', 'es6.modules'], | ||
| plugins: [HTMLBarsInlinePrecompile(precompile)], | ||
| filenameRelative: 'module-name-test.js' |
There was a problem hiding this comment.
Sorry to be such a complainer here, but I am currently not sure if the current test fully covers what the moduleName functionality does. Do you think there is a way we can use babel.transformFileSync or something, so it tests that the file.opts.filenameRelative is the correct value? I just want to make sure that if babel is transforming templates from a file, then we are reading the correct value. Using filenameRelative looks ok, but at the moment it looks to me like we are only stubbing the property and not ensuring that it is really the name of a file.
There was a problem hiding this comment.
sounds good, will update tomorrow.
There was a problem hiding this comment.
Thanks @jasonmit! Much appreciated!
|
@pangratz test updated to use |
| var fullpath = path.join(__dirname, 'fixtures', 'hello-world.js'); | ||
|
|
||
| var precompile = function(template, options) { | ||
| assert.equal(options.moduleName, fullpath); |
There was a problem hiding this comment.
I have just one more question: I have never used the moduleName option so forgive my ignorance, but since we're using relativeName, wouldn't we expect the moduleName be a relative path here? I have seen that there is the sourceRoot option for babel.transformFileSync, which might be the one to specify to which directory the file should be resolved relative to?
I am just asking. You said you used that successfully in one of your plugins and it works, so I am just asking because I want to understand what the moduleName option is used for...
There was a problem hiding this comment.
Ember uses it internally for hinting at deprecation warnings in a template. I use it for building scoped CSS modules.
As for the relativeName not being relative, it seems that way. The documentation says the opposite -- so I'm a bit confused. Lets hold off on merging until I can resolve this, we wouldn't want to leak full paths.
There was a problem hiding this comment.
Ember uses it internally for hinting at deprecation warnings in a template. I use it for building scoped CSS modules.
Thanks for the heads up!
Ok, waiting seems like a good idea. Thanks for looking into this!
There was a problem hiding this comment.
Did some digging, it looks like sourceRoot only knocks off the path when dealing with module transformations.
Instead of this PR, would you be for adding an optional second arg that has a precompilerOptions method who's return value is used to pass on to the precompiler?
HTMLBarsInlinePrecompilePlugin(precompile, {
precompileOptions(opts) {
// example of how someone might use it to construct their own moduleName
var sourceRootRegEx = new RegExp("^" + opts.sourceRoot + "/?");
return {
moduleName: opts.filenameRelative.replace(sourceRootRegEx, "")
};
}
});Not implementing the method will have the same behavior as today.
jasonmit
changed the title
| sourceRoot: sourceRoot, | ||
| plugins: [HTMLBarsInlinePrecompile(precompile, { | ||
| precompileOptions: function(opts) { | ||
| // example of how someone might use it to construct their own moduleName |
There was a problem hiding this comment.
or any future options that get added
|
😍 that is an awesome idea! I like that API. If you're in the mood, can you add a small section the README outlining this functionality? I guess this only needs a squash and then it's good to go. Thanks for bearing with me here. This turned out great! |
|
Agree, I like this more and moves the responsibility away from this plugin. I'll update the README shortly. |
|
README updated. Feel free to squash on merge (relative new GitHub feature). |
|
Replaced by #75. |
ember-cli-htmlbarsalso setsmoduleNameand other transformations may rely on this property.https://github.com/ember-cli/ember-cli-htmlbars/blob/master/index.js#L69
This PR brings the ability to implement a method to construct a
moduleNamebased on the babel options that the babel transform provides to plugins. It will also work with any future options that may be added to the precompiler.