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

Give an opportunity to the renderer to perform clean when disconnected #1244

Closed
freeman opened this issue Nov 2, 2017 · 6 comments
Closed

Comments

@freeman
Copy link
Contributor

freeman commented Nov 2, 2017

Not sure if it is a bug or a feature request ...

Current behaviour

When a custom element if removed from the DOM, the disconnectedCallback is properly called but this does not cascade to the preact components below. This means the componentWillUnmount lifecycle method is never called in those.

This can lead to memory leaks as for example any subscription taken in the componentDidMount lifecycle will not be released properly when the custom element is removed from the DOM.

Expected behaviour

skatejs should notify the renderer so that the proper lifecycle methods of the renderer can be correctly triggered.

@Hotell
Copy link
Contributor

Hotell commented Nov 2, 2017

Hmm interesting, thanks for report! maybe this should be opened directly on renderer-preact ?

@freeman
Copy link
Contributor Author

freeman commented Nov 2, 2017

You are right. Done. Feel free to close this one.

@Hotell Hotell closed this as completed Nov 2, 2017
@freeman
Copy link
Contributor Author

freeman commented Jan 3, 2018

Since the monorepo migration, I am not sure the fix that happened in the previous repository has been ported back. Maybe we should reopen this issue ?

@treshugart
Copy link
Member

Good point, @freeman. I think this can be done by providing a disconnectedCallback() in the renderer class. We should always be calling back to it anyways, and the default withComponent() implementation will automatically.

@treshugart
Copy link
Member

This is starting to make me wonder if there should be a base renderer-reactalike for all things that follow the same component "spec".

@treshugart
Copy link
Member

Using @skatejs/element you can now hook into the disconnectedCallback() to do this for all renderer subclasses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants