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

Batch attributeChangedCallback / Safeguard required props #33

Open
samyhrer opened this issue May 19, 2020 · 4 comments
Open

Batch attributeChangedCallback / Safeguard required props #33

samyhrer opened this issue May 19, 2020 · 4 comments

Comments

@samyhrer
Copy link

The setup:

  • A small preact-application wrapped in preact-custom-element, accepting three props which all are required by the preact-application.
  • The resulting custom element is dynamically created and updated inside a React-application

From my observations it does not seem like the wrapped preact-application is guaranteed to receive all props, with their corresponding values, in one pass. Even though they are provided to the custom element in "one go". I assume this has to do with how web components and the attributeChangedCallback work https://github.com/preactjs/preact-custom-element/blob/master/src/index.js#L28. Missing values for required props can of course be mitigated by having safeguards in place in the preact-application. But I want to discuss approches to include this functionality in this library instead

Thoughts/Questions:

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Sep 1, 2020

To me, batching sounds like the proper solution for this issue 👍

@raihle
Copy link

raihle commented Oct 9, 2020

Because connectedCallback is called as soon as one property is set, the component will be rendered before all required props have a value. This appears to be fixed by simply removing the call to connectedCallback from the property setters - it will be invoked by the browser anyways, seemingly after all properties have had a chance to be set. Some extra null-checking will be required here and there. What do you think?

Somewhat related, rerendering for every change could be bad for performance if many changes are made at once, and batching (even just deferring for 0 ms) could alleviate that. But maybe preact already does something about this?

@marvinhagemeister
Copy link
Member

@raihle Preact batches calls to setState, but not render()/hydrate(). So having batching in this adapter would be a welcome improvement as you outlined 👍

@CaffeinatedCodeMonkey
Copy link

Is there any update one this issue? I'm currently running into the same situation.

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

No branches or pull requests

4 participants