Navigation Menu

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

fix(custom-elements): ensure props are available before initial render (fix #4716)) #4792

Closed
wants to merge 1 commit into from

Conversation

LinusBorg
Copy link
Member

@LinusBorg LinusBorg commented Oct 13, 2021

This PR fixes a case where props with type "Number" lead to a premature first render - before the initial component props have been collected from the DOM element properties that have been set.

close #4716

asyncDef().then(resolve)
asyncDef()
.then(resolve)
.then(() => this._update())
Copy link
Member Author

@LinusBorg LinusBorg Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary to have the asyncWrapper's props updated before it renders the nested actual component.

Without this, the async Component test case fails now.

@LinusBorg LinusBorg linked an issue Oct 15, 2021 that may be closed by this pull request
@LinusBorg
Copy link
Member Author

Sidenote: I'm not sure yet if this also fixes #4789 as that one doesn't involve a number prop. But I couldn't identify another part of the code that could be responsible yet.

@matt-filion
Copy link

What's holding up this merge request getting approved?

@LinusBorg
Copy link
Member Author

Time

@yyx990803
Copy link
Member

Looks like this was already fixed by 3ca8317 since I can no longer reproduce #4716, even without this PR.

@yyx990803
Copy link
Member

Hmm, I may have missed something but this PR doesn't seem to affect either #4716 or #4789. But it does fix the added test case.

@yyx990803
Copy link
Member

I refactored the resolve call a bit more and I believe 4b7f76e should be a more efficient fix (it avoids some unnecessary _update() calls)

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.

Props not available in life-cycle hooks in custom elements
3 participants