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

Feature: Async component rendering #1668

Closed
wants to merge 2 commits into from

Conversation

EduApps-CDG
Copy link

Async component rendering

Objective 1
This PR aims to support async component rendering through the renderToString function.

Problem:

class RootElem extends Component {
  render() {
    return <>
        <div>Test</div>
        {this.props.children}
    </>
  }
}

class AsyncElem extends Component {
  async render() {
    // do something async...
    return <>
        <div>Async Elem</div>
    </>
  }
}

console.log(
  <RootElem>
    <AsyncElem />
  </RootElem>
);

In the example above, Inferno complains and throw a error because AsyncElem.render() is a Promise<...>. The PR changes renderVNodeToString(vNode, parent, context) to resolve vNode as soon as possible to continue the operation normally.

Objective 2
While changing the test cases, i found a specific test case which is not fullfilled in Security - SSR > renderToString > Should not render invalid tagNames.

Closes Issue
(no issue found)

@EduApps-CDG
Copy link
Author

Oh yeah, npm complains while installing dependencies. I had to use npm i --legacy-peer-deps to get it running.

@Havunen
Copy link
Member

Havunen commented Jun 3, 2024

Hi,

Thanks for the PR, but from where did you get the idea of making render method async? While I think renderToString could be async at the top level changing component render method async sounds like anti-pattern.
A better approach would be to keep the render method synchronous so there is always something to render and then do your async logic in the component lifecycle methods.

@Havunen Havunen closed this Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants