-
Notifications
You must be signed in to change notification settings - Fork 45
Remove the mutation observer feature #100
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
Remove the mutation observer feature #100
Conversation
|
@stephencookdev I had one thought about this today. We do have some situations where we've rendered: And then something updates it to be: We do need to make sure we deal with that case. I wonder if when you define a Reactive Element if you should give us a list of props that will change, and then we can observe and listen to changes using the native spec: https://developers.google.com/web/fundamentals/web-components/customelements#reactions - happy for this to be a separate PR. @ChristianMurphy does removing this feature cause problems for you? |
Why + where do we do that? Just trying to understand it as a feature fully |
Major issues, attribute updates and slot/children updates are being used.
HTML attributes are the equivalent of React props. |
|
Yeah, also wanted to advocate for "this is useful once you change props
from native DOM". But just curious... I remember there is an event
named attributesChanged on a custom element. Should it be used instead of
mutation observer?
чт, 14 февр. 2019 г. в 16:53, Christian Murphy <notifications@github.com>:
… does removing this feature cause problems for you?
Major issues, attribute updates and slot/children updates are being used.
Why + where do we do that?
HTML attributes are the equivalent of React props.
Anytime component state needs to be updated by it's parent, this is needed.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#100 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAxerxba6S5j1sI_0n7z7Ah820ymF2Jeks5vNYZsgaJpZM4a6DPf>
.
|
A limitation of the spec is that attributes that will change need to be explicitly defined in It would also make the API a bit clunky. Ideally |
|
Okay, I'm going to try messing about with this a bit to try and get the propagation of HTML attributes --> React prop updates working, without causing remounting bugs. |
We use it where we have Elm rendering a custom web element - rather than re-render from scratch, if props update Elm will just update the attributes in place, so we need to deal with that. For our use case just using Happy to chat through or have a think about this in person. |
The mutation observer idea is cool, but largely unnecessary - there shouldn't be much dynamism on the web-component side of your web-component/React bridge. The dynamic stuff should really all be living on the React side of stuff.
But more specifically, the code is a little buggy as things stand; when a component re-mounts, the code to determine the element's children does not calculate them correctly (possibly just if the remount is near the initial mount?), and instead determines the children array to always be
[]So it seems that the easiest fix is probably just to remove it entirely