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

Remove fetcher.type/fetcher.submission #5716

Merged
merged 4 commits into from
Mar 14, 2023

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Mar 8, 2023

Removes fetcher.type and fetcher.submission for v2. We still keep a small back-compat layer for now since there's an unintentional mismatch in casing between formMethod on Remix useFetcher/useTransition versus react Router useFetcher/useNavigation

@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2023

🦋 Changeset detected

Latest commit: 04b6ba6

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

This PR includes changesets to release 18 packages
Name Type
@remix-run/react Major
@remix-run/testing Major
create-remix Major
remix Major
@remix-run/architect Major
@remix-run/cloudflare Major
@remix-run/cloudflare-pages Major
@remix-run/cloudflare-workers Major
@remix-run/css-bundle Major
@remix-run/deno Major
@remix-run/dev Major
@remix-run/eslint-config Major
@remix-run/express Major
@remix-run/netlify Major
@remix-run/node Major
@remix-run/serve Major
@remix-run/server-runtime Major
@remix-run/vercel Major

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

@brophdawg11 brophdawg11 force-pushed the brophdawg11/remove-use-transition branch from af955aa to ccdb039 Compare March 10, 2023 17:07
@brophdawg11 brophdawg11 force-pushed the brophdawg11/remove-fetcher-type-submission branch from 4d3e2e8 to 5719710 Compare March 10, 2023 17:12
data,
formMethod: formMethod.toUpperCase() as FormMethod,
Copy link
Contributor Author

@brophdawg11 brophdawg11 Mar 10, 2023

Choose a reason for hiding this comment

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

Small back-compat layer to preserve the Remix uppercase FormMethod on useFetcher

@brophdawg11 brophdawg11 force-pushed the brophdawg11/remove-fetcher-type-submission branch from 9d92955 to b00a959 Compare March 13, 2023 16:05
Comment on lines +5 to +8
Remove back-compat layer for `useFetcher`/`useFetchers`. This includes a few small breaking changes:
* `fetcher.type` has been removed since it can be derived from other available information
* "Submission" fields have been flattened from `fetcher.submission` down onto the root `fetcher` object, and prefixed with `form` in some cases (`fetcher.submission.action` => `fetcher.formAction`)
* `<fetcher.Form method="get">` is now more accurately categorized as `state:"loading"` instead of `state:"submitting"` to better align with the underlying GET request
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are technically breaking since they happened in the same API and there's no flag that applied these changes. Is this something small enough we can call it out in the release notes? Or would it be worth a quick v2_fetcher flag to add this behavior into v1?

We did not have this issue on useTransition/useNavigation because it was a new hook introducing the changed behavior.

let fetcher: FetcherStates["Loading"] = {
state,
data,
formMethod: formMethod?.toUpperCase() as FormMethod,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still need this back-compat to align to the old uppercase format of formMethod. Separately we should figure out how to get RR to use uppercase so this can be removed, but that currently would be a breaking change in RR.

Base automatically changed from brophdawg11/remove-use-transition to v2 March 13, 2023 17:11
@brophdawg11 brophdawg11 force-pushed the brophdawg11/remove-fetcher-type-submission branch from b00a959 to 04b6ba6 Compare March 13, 2023 17:12
@brophdawg11 brophdawg11 merged commit 80be434 into v2 Mar 14, 2023
@brophdawg11 brophdawg11 deleted the brophdawg11/remove-fetcher-type-submission branch March 14, 2023 13:22
@MichaelDeBoey MichaelDeBoey added the BREAKING CHANGE This change will require a major version bump label Jul 9, 2023
@MichaelDeBoey MichaelDeBoey added the v2 Issues related to v2 apis label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump CLA Signed renderer:react v2 Issues related to v2 apis
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants