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

use options.unmount instead of overriding component.componentWillUnmount #2919

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

tanhauhau
Copy link
Contributor

@tanhauhau tanhauhau commented Jan 11, 2021

Following up to #2917, use options.unmount instead of overriding componentWillUnmount

this will check component._onResolve on every unmount though, not sure how much will this add up to slow things down

src/internal.d.ts Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jan 11, 2021

Coverage Status

Coverage increased (+0.003%) to 99.445% when pulling 0802e6d on tanhauhau:tanlh/use-unmount-options into 3efb2d0 on preactjs:master.

@developit
Copy link
Member

From the size bot:

Size Change: +29 B (0%)

Total Size: 42 kB

Filename Size Change
compat/dist/compat.js 3.35 kB +10 B (0%)
compat/dist/compat.module.js 3.34 kB +9 B (0%)
compat/dist/compat.umd.js 3.4 kB +10 B (0%)

compat/src/suspense.js Outdated Show resolved Hide resolved
Copy link
Contributor Author

@tanhauhau tanhauhau left a comment

Choose a reason for hiding this comment

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

I've added some test cases and relevant fixes from #2989

compat/src/suspense.js Outdated Show resolved Hide resolved
@JoviDeCroock JoviDeCroock merged commit 9250ac9 into preactjs:master Feb 9, 2021
@JoviDeCroock
Copy link
Member

Thank you so much for all the efforts @tanhauhau

This was referenced Mar 15, 2021
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.

None yet

5 participants