Support strict embroider builds #647
Conversation
| @@ -1,4 +1,4 @@ | |||
| {{#if this.useElementHelper}} | |||
| {{#if (macroDependencySatisfies 'ember-element-helper' '^0.6.1')}} | |||
There was a problem hiding this comment.
@mike-engel how would this work for ember-element-helper higher versions like v0.7 and v0.8?
There was a problem hiding this comment.
@mike-engel how would this work for ember-element-helper higher versions like v0.7 and v0.8?
Thanks for catching this. This was a lazy copy/paste from the getter 😅
There was a problem hiding this comment.
@SergeAstapov I wonder if this is a bug or just expected behavior? This matrix is out of date (I think), but macroDependencySatisfies has no runtime function. Additionally, the docs explicitly say that dead branches are only removed on production builds:
The macroCondition macro allows branch level code isolation (and deletion in the case of production builds)
So either there's a bug where ember classic builds (although, this is still happening on my embroider build, too) aren't actually replacing code, or this is as expected. I'd assume the former, but I've no idea how to test that theory.
There was a problem hiding this comment.
Huh, I guess macros without a runtime should always be pruning branches: embroider-build/embroider#1468
There was a problem hiding this comment.
It may be that embroider-build/embroider#1468 only fixed the javascript macroCondition and not the handlebars one. If so, we should fix that.
There was a problem hiding this comment.
My (maybe naive) fix for templates: embroider-build/embroider#1688
There was a problem hiding this comment.
Additionally, I think macroCondition (macroDependencySatisfies 'ember-element-helper' '>=0.6.1') should also work if that PR doesn't get accepted
|
@SergeAstapov is there anything I can help to get it merged and released? |
|
@villander this fix is not working, hence it's not ready to be merged |
|
@villander @SergeAstapov I've decided that macros are too buggy right now, and opened #657 to drop support for the old version of |
|
Closing in favor of #657 |
|
@mike-engel just to let you know we released your PR to embroider in https://github.com/embroider-build/embroider/releases/tag/v4.1.3-%40embroider%2Faddon-dev in case you want to try this approach again? |
Potential solution for #594, alternative for #633.
Should also fix #642.
This uses the
macroDependencySatisfieshelper and brings backensure-safe-component