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
Properly call preprocessTree
/ postprocessTree
for addons.
#6516
Properly call preprocessTree
/ postprocessTree
for addons.
#6516
Conversation
28f67eb
to
dd427a4
Compare
dd427a4
to
919bbac
Compare
@rwjblue can you confirm that master...nathanhammond:addon-templates-pre-post-process is a valid rebase of this PR? If so I'mma force push to your branch... |
Ya, looks good.
|
4a1c398
to
53524ba
Compare
☔ The latest upstream changes (presumably #6627) made this pull request unmergeable. Please resolve the merge conflicts. |
53524ba
to
3355697
Compare
Rebased, will get some basic tests added tomorrow morning. And then we will follow up with much better tests when #6449 lands. |
I'm pretty sure that this is not doing exactly what we want it to do. Here is the log of the output after this change given the test setup in #6449. My linking bug is why my test in #6611 passed originally. Before Change
After Change
|
☔ The latest upstream changes (presumably #6629) made this pull request unmergeable. Please resolve the merge conflicts. |
@nathanhammond - Ya, I noticed the same while working on the basic tests within the current system. Its just a boneheaded mistake, will push the update shortly. |
78208f7
to
7614746
Compare
This fixes an oversight that prevented addons from properly invoking the same `preprocessTree` / `postprocessTree` hooks that were added for apps. The implementations for `_addonPreprocessTree` and `_addonPostprocessTree` should be extracted and shared from `lib/broccoli/ember-app.js`.
7614746
to
8952331
Compare
preprocessTree
/ postprocessTree
for addons.preprocessTree
/ postprocessTree
for addons.
8952331
to
e68ea12
Compare
OK, finally got this rebased and updated with old school tests. I added the tests to the previously existing I believe this is in a good state to land... |
What I've done:
|
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've made a couple tiny tweaks rather than commenting on them.
@@ -597,8 +591,7 @@ EmberApp.prototype.addonPostprocessTree = function(type, tree) { | |||
@return {Tree} Processed tree | |||
*/ | |||
EmberApp.prototype.addonPreprocessTree = function(type, tree) { | |||
return this.project.addons.reduce((workingTree, addon) => | |||
(addon.preprocessTree ? addon.preprocessTree(type, workingTree) : workingTree), 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.
I'm glad this ternary is gone.
@homu r+ |
📌 Commit e561248 has been approved by |
…hanhammond Properly call `preprocessTree` / `postprocessTree` for addons. This fixes an oversight that prevented addons from properly invoking the same `preprocessTree` / `postprocessTree` hooks that were added for apps. TODO: - [x] The implementations for `_addonPreprocessTree` and `_addonPostprocessTree` should be extracted to a util shared with `lib/broccoli/ember-app.js`. - [x] Types - [x] `js` (run for `this.treePaths['addon']`) - [x] `css` (run for `this.treePaths['addon-styles']`) - [X] `template` (run for `this.treePaths['addon-templates']`) - [x] Tests for all types.
☀️ Test successful - status |
This fixes an oversight that prevented addons from properly invoking the same
preprocessTree
/postprocessTree
hooks that were added for apps.TODO:
_addonPreprocessTree
and_addonPostprocessTree
should be extracted to a util shared withlib/broccoli/ember-app.js
.js
(run forthis.treePaths['addon']
)css
(run forthis.treePaths['addon-styles']
)template
(run forthis.treePaths['addon-templates']
)