[Glimmer] Unskip component registration tests #13560
Conversation
| if (isEnabled('ember-glimmer')) { | ||
| OutletView = require('ember-glimmer/views/outlet').default; | ||
| } else { | ||
| OutletView = require('ember-htmlbars/views/outlet').OutletView; |
There was a problem hiding this comment.
ember-templates already does this branch
There was a problem hiding this comment.
nevermind, I thought it did, but I was wrong. I'm trying to get away from ember-views requiring ember-glimmer or ember-htmlbars, I'm trying to have it just be where stuff that is shared not branched is.
There was a problem hiding this comment.
if this is used from a test in packages/ember/tests only can you move it there?
There was a problem hiding this comment.
@krisselden Makes sense. I've moved the file into the ember package.
|
LGTM |
| if (component) { | ||
| let templateFullName = 'template:components/' + name; | ||
| let { | ||
| templateForName, |
There was a problem hiding this comment.
why do we mixin this into component and pull it off? can we just drop it
There was a problem hiding this comment.
@krisselden Htmlbars expects it to be on the view/component, but component init runs too late for the same logic to apply in glimmer. I was trying to keep things DRY by pulling templateForName off the class, but I can WET-ify that function if it's preferred.
There was a problem hiding this comment.
I think we should emit a different opcode or the dynamic opcode when a layout is late bound and assert on the layout property like the tag expression is asserted.
There was a problem hiding this comment.
maybe just use the partial opcode for the layout
|
☔ The latest upstream changes (presumably #13585) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Superseded by #13588. |
Title says it all.