solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.1k stars 137 forks source link

Form component #409

Open SarguelUnda opened 2 months ago

SarguelUnda commented 2 months ago

Summary

This PR does 2 things:

Consideration

To achieve this we use the same trick as action which is to globally register the form element in a Set. We register it by ref but we could consider to use solidjs createUniqueId if we prefere to use string instead of dom node.

Example usage

Example app: https://github.com/SarguelUnda/solidrouter-pr-form

import { Form, action } from '@solidjs/router';

const myAction = action(async (data: FormData) => {
  console.log("ACTION")
});

const MyForm = () => {
  return <Form action={myAction} method="post">
    <input name="foo" />
    <input name="bar" />
    <button type="submit">POST ME</button>
    <button name="getsubmit" type="submit" formMethod='get' formAction="newr?azeizaei=1" >GET ME</button>
  </Form>
}
changeset-bot[bot] commented 2 months ago

⚠️ No Changeset found

Latest commit: 54093aeb664332579fb78d4a4850a85ad38a1b74

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

SarguelUnda commented 2 months ago

Other points that might warrant discussion around this PR. The answer I'm giving are my own as a starter of conversation and any feedback on them is welcome

question: Why using a component and not extending the action api ? answer: We could extend the action api to register the form as a routable get form if instead of getting an async function it get a relative url as a string for instance. I didn't go toward this design mainly to not change an existing api, because I was not sure of all the implications. It also fell less natural to me on the userside of things but this is highly debatable. I can rewrite this pr if the other design is preferable

import { Form, action } from '@solidjs/router';

const myAction = action(async (data: FormData) => {
  console.log("ACTION")
});

const MyForm = () => {
  return <Form action={myAction} method="post">
    <input name="foo" />
    <input name="bar" />
    <button type="submit">POST ME</button>
    <button name="getsubmit" type="submit" formMethod='get' formAction="newroute" >GET ME</button>
  </Form>
}

// vs 

const MyForm = () => {
  return <form action={myAction} method="post">
    <input name="foo" />
    <input name="bar" />
    <button type="submit">POST ME</button>
    <button name="getsubmit" type="submit" formMethod='get' formAction={action("newroute")} >GET ME</button>
  </form>
}

question: Why the component is inside its own file and not with the other components ? answer: Data api as I understand is quite optional so I didn't want to write it within the global scope of the package.

SarguelUnda commented 1 week ago

Hello,

Any advice how to bring this proposition forward ?