feat(remix-react): add submitOptions argument for useSubmit and useFetcher

[kentcdodds]

This fixes an actual bug I was having when trying to use a fetcher in a useEffect. It's also a handy API that I've seen people ask for as well.

Closes: #4872

• Docs
• Tests

Testing Strategy: Automated Integration test

This is just pushing the failing test to demonstrate the issue.

The issue is actually technically correct behavior, but I need a new feature to accomplish what I want so I'll be changing the test once this new API is supported. I'm working on that next.

Implementation is finished and documented. Ready to discuss/merge.

Until this is merged, the only way to reliably use a fetcher.submit in a dep array is to do something like this:

const fetcher = useFetcher()
const submitRef = useRef(fetcher.submit)
useEffect(() => {
  submitRef.current = fetcher.submit
})
React.useEffect(() => {
  // can safely use submitRef without including in dep array
  submitRef({other, deps}, {action: '/some-path', method: 'post'})
}, [other, deps])

The drawback here is that if we actually do depend on the defaultAction then it's possible we'll get the wrong one when submit is called.

This PR fixes that by allowing users to specify an action (among other submit options):

const fetcher = useFetcher({action: '/some-path', method: 'post'})
React.useEffect(() => {
  // can safely include fetcher in dep array
  fetcher.submit({other, deps})
}, [other, deps, fetcher])

[changeset-bot]

🦋 Changeset detected

Latest commit: 354482b

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

This PR includes changesets to release 17 packages

Name	Type
remix	Minor
@remix-run/react	Minor
@remix-run/testing	Minor
create-remix	Minor
@remix-run/architect	Minor
@remix-run/cloudflare	Minor
@remix-run/cloudflare-pages	Minor
@remix-run/cloudflare-workers	Minor
@remix-run/deno	Minor
@remix-run/dev	Minor
@remix-run/eslint-config	Minor
@remix-run/express	Minor
@remix-run/netlify	Minor
@remix-run/node	Minor
@remix-run/serve	Minor
@remix-run/server-runtime	Minor
@remix-run/vercel	Minor

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

[machour]

Remember to target the dev branch 🙏

[kentcdodds]

I thought about making key a part of the options object in useSubmitImpl instead of accepting it as a first param, but decided to not cause a breaking change to useSubmitImpl because it's exported and it's possible people are using it. I actually don't hate this though so maybe it's fine?

[MichaelDeBoey]

Since I think we can safely remove the export (as it was never intended to be a public API), I think we can make key part of the submitOptions

[kentcdodds]

I actually kinda like that submitOptions is a separate argument, even though we have this funny key = undefined thing. But I don't care enough.

[kentcdodds]

Switched these to const and upper-case because otherwise typescript wasn't happy about default-assigning submitMethod to DEFAULT_METHOD below. Using const here allows typescript to narrow this down from a type of string to "get" which fits in the FormMethod type.

[kentcdodds]

By putting these calculations all here we can avoid having to pass defaultFormAction into the dependency array which is what fixes my original issue.

[kentcdodds]

Sorry, totally forgot to target dev. Thanks for the tip @machour. I've fixed it up.

[brophdawg11]

We're about to wipe this code out when we merge the 6.4 work 😬 - so if we're going to adopt this we probably need to do it in react-router-dom. Could we open a proposal for this API enhancement? Get enough upvotes and it could be accepted as soon as Monday!

[kentcdodds]

Yep. I expected as much. Didn't really think this would get merged as-is. Here's the discussion over there: remix-run/react-router#9737

[machour]

Closing this as it will be implemented in RR