Skip to content
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

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

chancancode
Copy link
Member

There was a lot of code that were needed for supporting named outlets, it seems like we never fully cleaned it up.

Plus dropping support for an old version of ember-test-helpers,
apparently.
@chancancode chancancode requested review from wagenet and ef4 November 9, 2023 06:46
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.
@chancancode chancancode force-pushed the named-outlets-removal branch from b0c8459 to 7f388fd Compare November 9, 2023 08:25
@@ -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"
Copy link
Contributor

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
Copy link
Contributor

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
*
Copy link
Contributor

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

Copy link
Member

@wagenet wagenet left a 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.

@chancancode chancancode merged commit f5572ac into main Nov 9, 2023
18 checks passed
@chancancode chancancode deleted the named-outlets-removal branch November 9, 2023 22:10
chancancode added a commit to emberjs/ember-test-helpers that referenced this pull request Nov 18, 2023
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>)`.
chancancode added a commit that referenced this pull request Nov 22, 2024
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
chancancode added a commit that referenced this pull request Nov 22, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants