Shopify / shopify-app-js

MIT License
280 stars 111 forks source link

AppProxyForm does not handle ref attribute #1605

Open ngbrown opened 2 weeks ago

ngbrown commented 2 weeks ago

Issue summary

Before opening this issue, I have:

The documentation says the <AppProxyForm/> component can be used in place of Remix's <Form/> but it does not correctly handle setting the ref attribute.

Expected behavior

What do you think should happen?

Accept the ref attribute and enable calling form functions such as form.current.submit().

Actual behavior

What actually happens?

Type '{ children: Element; ref: RefObject; method: "POST"; onSubmit: (ev: FormEvent) => void; onReset: () => void; }' is not assignable to type 'IntrinsicAttributes & AppProxyFormProps'. Property 'ref' does not exist on type 'IntrinsicAttributes & AppProxyFormProps'.ts(2322)

The component does not use forwardRef as it should.

https://github.com/Shopify/shopify-app-js/blob/08627d858b2fcdbaa8c200e29fb66b8b5928c06a/packages/apps/shopify-app-remix/src/react/components/AppProxyForm/AppProxyForm.tsx#L63-L79

Line 63 should be:

export const AppProxyForm = forwardRef(function AppProxyForm(props: AppProxyFormProps, ref?: LegacyRef<HTMLFormElement>) {

Line 75 should be:

<Form action={context.formatUrl(action, false)} {...otherProps} ref={ref}>

Steps to reproduce the problem

const form = useRef<HTMLFormElement>(null);
return (
  <AppProxyForm ref={form} action="/app/settings">
  </AppProxyForm>
);

Debug logs

N/A

lizkenyon commented 2 weeks ago

Hi there 👋

Thanks for flagging, and taking a detailed look into the problem. If you would like to feel free to put up a PR for this issue, and the team will prioritize reviewing it. Otherwise I will add this ticket to the backlog of work for us to resolve.

Thanks again!