Skip to content
This repository has been archived by the owner on Feb 6, 2021. It is now read-only.

Provide template compiler cache key to babel plugin. #34

Merged
merged 1 commit into from
Aug 10, 2016

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 10, 2016

This uses the file contents as cache key for the babel-plugin-htmlbars-inline-precompile plugin used.

This is based on the following PR's:

var PrecompileInlineHTMLBarsPlugin = HTMLBarsInlinePrecompilePlugin(Compiler.precompile); // jshint ignore:line
var templateCompilerFullPath = require.resolve(templateCompilerPath);
var templateCompilerCacheKey = fs.readFileSync(templateCompilerFullPath, { encoding: 'utf-8' });
var PrecompileInlineHTMLBarsPlugin = HTMLBarsInlinePrecompilePlugin(Compiler.precompile, templateCompilerCacheKey); // jshint ignore:line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we pass a checksum? it doesn't really matter, the file content and the checksum end up representing the exact same thing. I could go either way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require('crypto').createHash('sha1').update(templateCompiler).digest('hex')

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or do we expect the other side to do this. Also, should we use the entire second arg for this, or should we do:

new HTMLBarsInlinePrecompilePlugin(Compiler.precompile, { cacheKey: /* ... */ })

As it may be more self documenting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I will update to options hash.

RE: checksum vs content, it doesn't much matter to me, I'm happy to go either way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE: checksum vs content, it doesn't much matter to me, I'm happy to go either way.

lets leave it as is, it gets checksumed in the plugin serialization phase anyways correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use options hash.

@rwjblue rwjblue force-pushed the provide-cache-key-to-babel-plugin branch from 2e170d0 to 0e53540 Compare August 10, 2016 14:03
@@ -41,7 +41,7 @@
"ember-cli"
],
"dependencies": {
"babel-plugin-htmlbars-inline-precompile": "0.0.5",
"babel-plugin-htmlbars-inline-precompile": "rwjblue/babel-plugin-htmlbars-inline-precompile#cache-keys",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this for testing, will need to be updated to ^0.1.0 before releasing.

This uses the file contents as cache key for the
`babel-plugin-htmlbars-inline-precompile` plugin used.
@rwjblue rwjblue force-pushed the provide-cache-key-to-babel-plugin branch from 0e53540 to 0ec4aa1 Compare August 10, 2016 16:27
@rwjblue rwjblue merged commit 0c60518 into ember-cli:master Aug 10, 2016
@rwjblue rwjblue deleted the provide-cache-key-to-babel-plugin branch August 10, 2016 16:32
@@ -37,7 +38,11 @@ module.exports = {
global.EmberENV = EmberENV;

var Compiler = require(templateCompilerPath);
var PrecompileInlineHTMLBarsPlugin = HTMLBarsInlinePrecompilePlugin(Compiler.precompile); // jshint ignore:line
var templateCompilerFullPath = require.resolve(templateCompilerPath);
var templateCompilerCacheKey = fs.readFileSync(templateCompilerFullPath, { encoding: 'utf-8' });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont think this is sufficient, the templateCompiler may change on each rebuild, this merely checks the cache on start.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, but upon restart it will blow the async cache properly (which was not true previously). So its a tad better, but could use more work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, its better. Issue opened to track and fix :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants