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

[@shopify/react-form] form.submitErrors is not populated when returning {status: "fail"} #2500

Closed
2 tasks done
jokull opened this issue Dec 8, 2022 · 10 comments · Fixed by #2637
Closed
2 tasks done
Labels
Type: Bug 🐛 Something isn't working

Comments

@jokull
Copy link

jokull commented Dec 8, 2022

Overview

In a very simple example submitErrors is not populated.

Consuming repo

What repo were you working in when this issue occurred?

Made a reproducable example. https://stackblitz.com/edit/vitejs-vite-qctwdk?file=src/App.tsx


Checklist

  • Please delete the labels section before submitting your issue
  • I have described this issue in a way that is actionable (if possible)
@jokull jokull added the Type: Bug 🐛 Something isn't working label Dec 8, 2022
@jokull
Copy link
Author

jokull commented Dec 22, 2022

I now understand what is happening. Inside the useSubmit hook the submit promise itself is early-returning because the useMountedRef ref has turned false.

I'm not sure what the purpose of this early exit is. I would vote to remove it.

The mounted ref flips to false as soon as I change the form state (on the first field onChange). Maybe this is related to React 18 changes?

@jamalsoueidan
Copy link

jamalsoueidan commented Jan 7, 2023

I used some time to figure out it's not working in react 18!

Return submiterrors = []

import { FormErrors } from "@components/FormErrors";
import {
  Button,
  Card,
  Form,
  FormLayout,
  Frame,
  Page,
  TextField,
} from "@shopify/polaris";
import { useField, useForm } from "@shopify/react-form";

export default () => {
  const {
    fields: { phone },
    submit,
    submitErrors,
  } = useForm({
    fields: {
      phone: useField(""),
    },
    onSubmit: async (fieldValues) => {
      return {
        status: "fail",
        errors: [{ field: ["phone"], message: "bad form data" }],
      };
    },
  });

  console.log(submitErrors);

  return (
    <Frame>
      <div className="inline-block align-middle">
        <Page
          narrowWidth
          title="Receive password by phone"
          breadcrumbs={[{ content: "Login", url: "/" }]}
        >
          <FormErrors errors={submitErrors} />
          <Card sectioned>
            <Form onSubmit={submit}>
              <FormLayout>
                <TextField label="Phone" autoComplete="phone" {...phone} />

                <Button submit>Receive password</Button>
              </FormLayout>
            </Form>
          </Card>
        </Page>
      </div>
    </Frame>
  );
};

@jamalsoueidan
Copy link

I now understand what is happening. Inside the useSubmit hook the submit promise itself is early-returning because the useMountedRef ref has turned false.

I'm not sure what the purpose of this early exit is. I would vote to remove it.

The mounted ref flips to false as soon as I change the form state (on the first field onChange). Maybe this is related to React 18 changes?

Did you have a solution to this problem?

@jokull
Copy link
Author

jokull commented Jan 7, 2023

I use patch-package with this diff:
https://gist.github.com/jokull/fb17f617c3f449ffec6c2012a02751b0

@jokull
Copy link
Author

jokull commented Mar 21, 2023

This is fixed in Next.js 13.2. I suggest closing this. Here's a stackblitz to confirm.

@jamalsoueidan
Copy link

it's still a problem, strict mode is breaking it!

@QuintonC
Copy link
Contributor

@jamalsoueidan @jokull There is a linked pull request #2637 which has snapshot versions included. I've included them here in this comment as well for your convenience.

If you're able to test these snapshot versions and confirm if the issue is resolved for you I would appreciate it 🙂

You might need to install both, but hopefully the package manager of your choosing resolves the snapshot version of @shopify/react-hooks as well.

yarn add @shopify/react-form@0.0.0-snapshot-20230425154318
yarn add @shopify/react-hooks@0.0.0-snapshot-20230425154318

Thanks in advance!

@jokull
Copy link
Author

jokull commented Apr 26, 2023

LGTM! (BTW I might have spoken too soon on it being fixed back there in March - I think I just didn't have strict mode toggled on).

@jamalsoueidan
Copy link

@jokull you have time to test it? Since I don't :(

@jokull
Copy link
Author

jokull commented Apr 27, 2023

I reproduced the error with with strict mode turned off and on. Confirmed that it was indeed strict mode in dev that was causing the issue. The fix from @QuintonC WORKS. See here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐛 Something isn't working
Projects
None yet
3 participants