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

ENH Refactor Element to function component #1171

Merged

Conversation

emteknetnz
Copy link
Member

Issue #1164

I'm splitting the linked issue into two PRs

  • Firstly I'll do this one that only refactors the existing class component to a function component
  • Once this is merged, I'll do a second PR to do the GraphQL refactor

Note that I've removed the getDerivedStateFromError() / childRenderingError functionality because:

  • I didn't find a replacement for it using functional components
  • We don't use getDerivedStateFromError() in any other components, so I think for consistency we don't actually want it

@emteknetnz emteknetnz force-pushed the pulls/5/functional-component branch from 9561f73 to e3dd091 Compare April 18, 2024 05:52
@GuySartorelli
Copy link
Member

GuySartorelli commented Apr 21, 2024

I am reliably getting this error in the developer console whenever I expand an inline-editable block and then click "save" in the page (note: NOT when I click the ... menu and save the block from there)

Uncaught (in promise) Invariant Violation: Store reset while query was in flight (not completed in link chain)

Other than that it seems to work as expected locally. Haven't taken a proper look at the code yet but at a glance it looks okay.

@emteknetnz
Copy link
Member Author

emteknetnz commented Apr 21, 2024

That's an existing console error unrelated to this PR. It's coming from Apollo.

@GuySartorelli
Copy link
Member

I had originally requested replacing getDerivedStateFromError() but it looks like error boundaries are no longer directly supported in react. While I think losing that functionality isn't desirable, keeping it is probably more effort than it's worth right now.

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

LGTM

@GuySartorelli GuySartorelli merged commit e5cc486 into silverstripe:5 Apr 21, 2024
13 checks passed
@GuySartorelli GuySartorelli deleted the pulls/5/functional-component branch April 21, 2024 23:04
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.

2 participants