-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add addon.postprocessTree. #1214
Conversation
@stefanpenner - Before I update |
👍 |
@jakecraige - This should allow you do do what we talked about in IRC (post build integration with Cordova). Can you review an confirm? |
|
||
return addon.treeFor(type); | ||
}, this); | ||
EmberApp.prototype.addonPostprocessTree = function(type, tree) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're passing in type here but not using it. Is there a reason for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intend to have 'all', 'js', and 'css', then move the minification all into addons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But for now, I just added 'all' (to get in the door with something useful). Having the param there for now, means I don't have to break the API when I do flesh it out more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With that being said, should it be passed into the addon now since that's the plan? So the addons won't have to be updated when it changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oiy. You are correct. I forgot to pass it. Good catch, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now, thanks again for catching it.
@rjackson this is actually different from what I need for my use. I need to hook into the post rebuild process is well. I can talk it over more in IRC if you want. I could put together a PR if it seems useful for other people. I want to hook into the builder's .build method which sane uses and chain on some extra promises. If it needs to be non-blocking they can just use an immediate Promise.resolve edit: |
@jakecraige - You definitely can do what you are looking for with this API: function postprocessTree(type, tree) {
return {
read: function(readTree) {
return readTree(tree).then(function(srcDir) {
runMyAwesomePostBuildCommand();
return srcDir;
}
}
}
} That said, your PR makes it simpler for your use case for sure. |
@rjackson From the looks of it, doesn't this only run after the initial build? Not after rebuilds? |
@jakecraige - Nope, the |
@rjackson I guess I was logging in the wrong place when I was looking at it, lol. Good to know, thanks |
I do think we need both, one for broccoli based post-processing (like minification, fingerprinting, etc), and the other as a more general purpose post-build thing (which #1215 does well). |
OK, so the updated @stefanpenner - This is ready to go if you are +1. |
needs rebase now, but LGTM |
This hook allows any addon to hook into the final stage of the `toTree` process. Making things like `broccoli-uglify` and `broccoli-asset-rev` able to be addons and not need to live in the core of EmberCLI. Also: * Allows additional trees to be passed to `app.toTree`. Creating additional trees in the application `Brocfile.js` and merging them with the result of `toTree` is pretty common. Unfortunately, this means that any additional assets you have added will not be available for the post processing in addons. For example, any assets you add this way will never be fingerprinted. * As the addon API develops it is odd that `included` and `treeFor` are required to be implemented since many plugins will never interact with the `EmberApp` stuff (for commands, preprocessors, middlewares, blueprints (soon), etc). This PR removes the errors from being thrown. * Adds `broccoli-asset-rev` as an application dependency for newly blueprinted apps.
This hook allows any addon to hook into the final stage of the
toTree
process. Making things like
broccoli-uglify
andbroccoli-asset-rev
able tobe addons and not need to live in the core of EmberCLI.
Also:
app.toTree
. Creatingadditional trees in the application
Brocfile.js
and merging themwith the result of
toTree
is pretty common. Unfortunately, thismeans that any additional assets you have added will not be available
for the post processing in addons. For example, any assets you add
this way will never be fingerprinted.
included
andtreeFor
arerequired to be implemented since many plugins will never interact with
the
EmberApp
stuff (for commands, preprocessors, middlewares,blueprints (soon), etc). This PR removes the errors from being thrown.
broccoli-asset-rev
as an application dependency for newlyblueprinted apps.
broccoli-asset-rev
was updated in ember-cli/broccoli-asset-rev#8 and published as 0.0.10.