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

feat: Require init.input or init.query. #104

Merged
merged 4 commits into from
Dec 15, 2023
Merged

Conversation

stephenh
Copy link
Contributor

@stephenh stephenh commented Dec 8, 2023

We've had several auto-saving form-states not realize they should use init.input + map to get the latest data from the mutation response automatically included in the form.

In general, having init: T is a legacy/rare use case, so let's try removing it.

Callers that were doing init: data can now do init { input: data } although technically the init.input will pull in changes to data as its identity changes, while init: data would not.

If we roll this out and find a lot of places that need that init: data behavior, we could add something like:

  init: {
    input: data,
    initOnlyOnce: true,
  }

Or something like that, to be more explicit that they're requesting to not re-read the input: data value on each re-render.

We've had several auto-saving form-states not realize they should use
init.input + map to get the latest data from the mutation response
automatically included in the form.

In general, having `init: T` is a legacy/rare use case, so let's try
removing it.

Callers that were doing `init: data` can now do `init { input: data }`
although technically the `init.input` will pull in changes to `data`
as its identity changes, while `init: data` would not.

If we roll this out and find a lot of places that need that `init: data`
behavior, we could add something like:

```
  init: {
    input: data,
    initOnlyOnce: true,
  }
```

Or something like that, to be more explicit that they're requesting
to not re-read the `input: data` value on each re-render.
@stephenh stephenh requested review from bdow and TylerR909 December 8, 2023 19:26
* The opts has for `useFormState`.
*
* @typeparam T the form type, which is usually as close as possible to your *GraphQL input*
* @typeparam I the *form input* type, which is usually the *GraphQL output* type, i.e. the type of the response from your GraphQL query
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TylerR909 per your request. Fwiw I acknowledge the "form input type is the graphql output type" and "graphql input type is the form type" is awkward, but it seems to fundamentally step from:

  • GraphQL output type ==> form "input" ==> form "output" ==> GraphQL input

Open to better ways of naming/articulating this.

@stephenh stephenh merged commit 2e97582 into main Dec 15, 2023
2 checks passed
@stephenh stephenh deleted the feat-require-input-or-query branch December 15, 2023 16:25
homebound-team-bot pushed a commit that referenced this pull request Dec 15, 2023
## [2.24.0](v2.23.2...v2.24.0) (2023-12-15)

### Features

* Require init.input or init.query. ([#104](#104)) ([2e97582](2e97582))
@homebound-team-bot
Copy link

🎉 This PR is included in version 2.24.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants