-
-
Notifications
You must be signed in to change notification settings - Fork 407
renderComponent() #1099
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
base: master
Are you sure you want to change the base?
renderComponent() #1099
Conversation
|
||
When isInteractive is false, modifiers don't run, when true, modifiers do run. | ||
|
||
#### `into` |
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.
we could add after
to allow app rendering after some node, without creating extra element wrapper
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've never heard of folks doing this -- is there a compelling reason / use case?
(maybe a bunch of render components in a list?)
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 kinda want to push back on this, as I don't know if the current render can do this.
It can always be added later if we need it
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.
@NullVoxPopuli, I agree.
Main use-case is render ember app in the middle of smf
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.
smf?
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.
@NullVoxPopuli in between nodes, for example, we have:
<html>
<body>
<header> ama header </header>
{ HERE_I_WANT_TO_RENDER_WHOLE_APP without creating extra node }
<footer> ama footer </footer>
</body>
</html>
I'm a big fan of this, as this is the baseline to write a storybook adapter for gts files. But I have some questions around this, as I think the narrative is not clear (yet):
|
aye! the
what do you mean no backing class? This is covered in RFC#931 import { template } from "@ember/template-compiler";
class Example extends Component {
static {
template(
"Hello {{message}}",
{
component: this,
scope: () => ({ message }),
},
);
}
}
I only included it because the sample implementation from @wycats included it -- I'm more than happy to remove it, as it does feel redundant, since you need a handle to that same element (it is the |
/** | ||
* Renders a component into an element, given a valid component definition. | ||
*/ | ||
export function renderComponent( |
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.
wondering if it make sense to give function name just "render", because we can't render anything except component (or template-only component) in ember (app has it's own render logic)
|
||
#### `into` | ||
|
||
The element to render the compnoent into. |
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.
should we have body
as default target node?
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.
can we rename it to parentElement
to make it match what the DOM has already defined?
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.
we removed parentElement 🙈
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.
should we have body as default target node?
idk, any downsides?
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.
you removed the getter from the result. I see into
is ambivalent. It can mean, that this is a parent or replaced with the contents you provide. parentElement
is a concept, that is already known.
const render = modifier((element) => { | ||
let result = renderComponent(Demo, { | ||
owner, | ||
env: { document: document, isInteractive: true }, |
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.
for non-interactive envs we may have different function like renderSSR
, so, wondering if we really need isInteractive
flag, any use-cases for it?
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.
great question -- I'd need to dig in to glimmer-vm to see what all isInteractive is used for.
I don't know how much benefit there is to having renderComponent do multiple things via this flag vs your suggestion of renderSSR (which I like)
text/1099-renderComponent.md
Outdated
* Destroys the render tree and removes all rendered content from the element rendered into. | ||
*/ | ||
destroy(): void | ||
|
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 think render result should contain component instance itself, to be able to update it's properties if needed (and to be able to debug it)
/** | ||
* Optional owner. Defaults to `{}`, can be any object, but will need to implement the [Owner](https://api.emberjs.com/ember/release/classes/Owner) API for components within this render tree to access services. | ||
*/ | ||
owner?: object; |
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.
but will need to implement the Owner API
So should this be owner?: Owner;
then? And how can it default to {}
, which obvisouly does not implement Owner
?
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.
the owner doesn't need any specific methods implemented if no dependency injection is used.
in this use case for rendering, the owner is more about wiring up lifetimes, than it is adding resolving mechanisms.
text/1099-renderComponent.md
Outdated
|
||
## How we teach this | ||
|
||
`renderComponent` is meant as an integration-enabling API and would not be added to the guides, or need any documentation beyond what would live in source on the function itself. |
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.
TODO: add some examples
- REPL
- astro
- vitepress
- etc, etc
text/1099-renderComponent.md
Outdated
/** | ||
* Defaults to globalThis.document. | ||
*/ | ||
document?: SimpleDocument | Document; |
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.
TODO: investigate what all is used on the document -- can the interface be more minimal?
text/1099-renderComponent.md
Outdated
There is an existing implementation in [PR#20781](https://github.com/emberjs/ember.js/pull/20781/) | ||
|
||
Here is where this RFC differs: | ||
- no hasDOM (we always have a DOM, as we have not done any design on [Swappable Renderers](https://github.com/emberjs/ember.js/issues/20648)) |
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.
TODO: add this back, but default to true
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.
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.
explain that removing this involves legacy SSR, and we'd probably want to explore how modern SSR works
text/1099-renderComponent.md
Outdated
|
||
The args to pass to the component | ||
|
||
### Behaviors |
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.
create a renderer if one doesn't already exist.
otherwise use the host app's existing renderer (and revalidation stuff, etc)
Propose
renderComponent()
Rendered
Summary
This pull request is proposing a new RFC.
To succeed, it will need to pass into the Exploring Stage, followed by the Accepted Stage.
A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.
An FCP is required before merging this PR to advance to Accepted.
Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.
Exploring Stage Description
This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.
An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an
Exploring
label applied.An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.
Accepted Stage Description
To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.
If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.
When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.
Checklist to move to Exploring
S-Proposed
is removed from the PR and the labelS-Exploring
is added.Checklist to move to Accepted
Final Comment Period
label has been added to start the FCP