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.
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.
@@ -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/[email protected] 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>)`.
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