-
-
Notifications
You must be signed in to change notification settings - Fork 4.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
[CLEANUP] Remove all traces of named outlets code #20570
Conversation
Plus dropping support for an old version of ember-test-helpers, apparently.
The split definitions between the rendering and routing layer was because the rendering layer was the first package to be converted into TypeScript. Now that everything is converted they should be sharing the same definition of the interface.
b0c8459
to
7f388fd
Compare
@@ -120,16 +117,9 @@ function stateFor(ref: Reference, outlet: OutletState | undefined): OutletDefini | |||
let template = render.template; | |||
if (template === undefined) return null; | |||
|
|||
// this guard can be removed once @ember/test-helpers@1.6.0 has "aged out" |
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.
1.6.0 😅
// And interface reflects that. Now that this is not supported anymore, each | ||
// route implicitly renders into its immediate parent's `{{outlet}}` (no name). | ||
// Keeping around most of these to their appropriately hardcoded values for the | ||
// time being to minimize churn for external consumers, as we are about to rip |
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.
🎉
* Has to do with render helper and orphan outlets. | ||
* Whether outlet state was rendered. | ||
* @deprecated | ||
* |
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 really appreciate all these comments -- as someone who is just learning how the glue fits together, this is immensely helpful for provided much needed context as well as a bit of why
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.
This looks reasonable, but I can't say that I 100% understand all of it. If tests pass, that is encouraging.
The canary build started failing after emberjs/ember.js#20570 was merged. We were never supposed to pass a `TemplateFactory` to `OutletState`, but it was previously tolerated. The pattern was fixed in the pass, but regressed when implementing support for `render(<template>...</template>)`.
The actual behavior change was marked with an intimate deprecation and has aged out. The actual versions of ember-test-helpers affected seems to be around <= 3.2.0, which should be fully absorbed by now. The other changes to `OutletState` doesn't actually change behavior, those fields were mostly just kept around assuming we are keeping this code frozen and to be deleted soon, so they documented what those were on the off chance that someone came into look. Now that it looks like this code may be here to stay for a bit longer, it's worth cleaning it up to make things less confusing for core devs. Follow-up to #20570
The actual behavior change was marked with an intimate deprecation and has aged out. The actual versions of ember-test-helpers affected seems to be around <= 3.2.0, which should be fully absorbed by now. The other changes to `OutletState` doesn't actually change behavior, those fields were mostly just kept around assuming we are keeping this code frozen and to be deleted soon, so they documented what those were on the off chance that someone came into look. Now that it looks like this code may be here to stay for a bit longer, it's worth cleaning it up to make things less confusing for core devs. Follow-up to #20570
There was a lot of code that were needed for supporting named outlets, it seems like we never fully cleaned it up.