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

Actions: support React 19 useActionState() with progressive enhancement #11074

Merged
merged 23 commits into from
May 21, 2024

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented May 16, 2024

Changes

Demo: https://www.loom.com/share/e77b00743de4454697f2c6b6d16492c0?sid=ff1d3574-80f6-416e-9ba9-a63a62821881

Update Astro core and the Reaction integration to support useActionState() with progressive enhancement.

  • Pass down the action result on a new reactServerActionResult property. This is attached to component metadata to pass to our React renderer.
  • Update the React renderer to pass reactServerActionResult to the formState property. This is used to progressively enhance useActionState() calls.
  • Expose new withState() and getActionState() utilities from @astrojs/react/actions.
    • withState() - update an action's type signature and preserve progressive enhancement info for useActionState().
    • getActionState() retrieve the state value from your action handler.

Testing

Add e2e tests for server and client rendered like buttons with useActionState()

Docs

Changeset will suffice for now. TODO: update RFC to include React 19 usage examples

Copy link

changeset-bot bot commented May 16, 2024

🦋 Changeset detected

Latest commit: 93480e7

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: react Related to React (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) docs pr A PR that includes documentation for review labels May 16, 2024
@bholmesdev
Copy link
Contributor Author

bholmesdev commented May 16, 2024

Thoughts on the experimental_ prefix? It is coming from a new module file (@astrojs/react/actions), so we could say it's namespaced well enough to ditch the prefix.

@bholmesdev
Copy link
Contributor Author

(test failures are from my writing bad tests it seems. Will address tomorrow 😓 )

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I am concerned about having React stuff inside our runtime and types. I am not very fond of the change, I hope there are plans to make this generic, and let the renderer take care of it.

packages/astro/src/runtime/server/astro-island.ts Outdated Show resolved Hide resolved
packages/astro/src/runtime/server/hydration.ts Outdated Show resolved Hide resolved
packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
@@ -22,6 +24,16 @@ function toActionProxy(actionCallback = {}, aggregatedPath = '/_actions/') {
data,
}
};
action.safe.$$FORM_ACTION = function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

This part is very small and probably can't be moved out. So I don't see any issue with having this part.

@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label May 20, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@bholmesdev bholmesdev requested a review from matthewp May 20, 2024 21:32
@@ -314,8 +317,10 @@ export class RenderContext {
renderers,
resolve,
response,
request: this.request.clone(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be done conditionally only when necessary? Cloning every request would be expensive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a feeling this is slowing the test suite too. The clone() was a precaution but it shouldn't be necessary? I'll try just removing the clone(). I wouldn't know a conditional to check 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that doesn't work, an alternative would be to just pass requestFormData when a form data content type is present. This would scope down to the info we actually need to access from the React integration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed that clone was the slowdown. Good to know the impact that has

@bholmesdev bholmesdev dismissed github-actions’s stale review May 21, 2024 14:55

Merging into base branch, not main. Safe to merge

@bholmesdev bholmesdev merged commit 170722c into feat/react-19-basic-actions May 21, 2024
14 checks passed
@bholmesdev bholmesdev deleted the feat/react-19-action-state branch May 21, 2024 14:55
ematipico added a commit that referenced this pull request May 22, 2024
* deps: react 19

* feat: react progressive enhancement with useActionState

* refactor: revert old action state implementation

* feat(test): react 19 action with useFormStatus

* fix: remove unused context arg

* fix: wrote actions to wrong test fixture!

* deps: revert react 19 beta to 18 for actions-blog fixture

* chore: remove unused overrides

* chore: remove unused actions export

* chore: spaces vs tabs ugh

* chore: fix conflicting fixture names

* chore: changeset

* chore: bump changeset to minor

* Actions: support React 19 `useActionState()` with progressive enhancement (#11074)

* feat(ex): Like with useActionState

* feat: useActionState progressive enhancement!

* feat: getActionState utility

* chore: revert actions-blog fixture experimentation

* fix: add back actions.ts export

* feat(test): Like with use action state test

* fix: stub form state client-side to avoid hydration error

* fix: bad .safe chaining

* fix: update actionState for client call

* fix: correctly resume form state client side

* refactor: unify and document reactServerActionResult

* feat(test): useActionState assertions

* feat(docs): explain my mess

* refactor: add experimental_ prefix

* refactor: move all react internals to integration

* chore: remove unused getIslandProps

* chore: remove unused imports

* chore: undo format changes

* refactor: get actionResult from middleware directly

* refactor: remove bad result type

* fix: like button disabled timeout

* chore: changeset

* refactor: remove request cloning

* Update .changeset/gentle-windows-enjoy.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* changeset grammar tense

---------

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr A PR that includes documentation for review pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: react Related to React (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants