Skip to content

fix edge cases for thrown promises in Suspense#2776

Merged
marvinhagemeister merged 4 commits intopreactjs:masterfrom
kitten:fix/suspense-hooks-edgecases
Sep 27, 2020
Merged

fix edge cases for thrown promises in Suspense#2776
marvinhagemeister merged 4 commits intopreactjs:masterfrom
kitten:fix/suspense-hooks-edgecases

Conversation

@kitten
Copy link
Copy Markdown
Contributor

@kitten kitten commented Sep 25, 2020

I caught these edge cases while reading preact-ssr-prepass and hooks/src/index.js since I ran into them in react-ssr-prepass as well. While maybe a little more unexpected, some users may expect that they can throw promises in useMemo or even in useState initialisers.

This means that the hook implementation has to take into account that useMemo's factory function or useState's setup function may throw before setting up the hook state. Minor changes though;

  • Flipped assignment in useReducer (hookState._value before hookState._component)
  • Flipped assignments in useState (state._value before state._args)

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 25, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 2cb903f on kitten:fix/suspense-hooks-edgecases into 27eca03 on preactjs:master.

Copy link
Copy Markdown
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

Nice work 💯

Copy link
Copy Markdown
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Love this! Thank you so much for making a PR 🙌

@marvinhagemeister marvinhagemeister merged commit b3da47d into preactjs:master Sep 27, 2020
@kitten kitten deleted the fix/suspense-hooks-edgecases branch September 27, 2020 12:02
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.

4 participants