[Solid.js] feat: add withForm and createAppForm APIs #1453
Conversation
|
View your CI Pipeline Execution ↗ for commit 1a851ee.
☁️ Nx Cloud last updated this comment at |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1453 +/- ##
==========================================
+ Coverage 88.83% 93.15% +4.31%
==========================================
Files 31 3 -28
Lines 1379 73 -1306
Branches 347 4 -343
==========================================
- Hits 1225 68 -1157
+ Misses 137 5 -132
+ Partials 17 0 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
crutchcorn
changed the title
| }) as Component<ParentProps> | ||
|
|
||
| const AppField = ((props) => { | ||
| const { children, ...rest } = props |
There was a problem hiding this comment.
https://docs.solidjs.com/reference/reactive-utilities/split-props
Use this instead. children is a function so it would be an edge case for someone to do reactive() ? () => Comp1 : () => Comp2 but still.
| TOnServer extends undefined | FormAsyncValidateOrFn<TFormData>, | ||
| TSubmitMeta, | ||
| TRenderProps extends Record<string, unknown> = {}, | ||
| >({ |
There was a problem hiding this comment.
would avoid destructure here in case the user wants to make them reactive too. The deeper reactivity within props and render are maintained so idk if we want to even enable that pattern.
| const TFormComponents extends Record<string, Component<any>>, | ||
| >({ | ||
| fieldComponents, | ||
| fieldContext, |
There was a problem hiding this comment.
Same with the destructures here, I would avoid them
TODOs
useFieldContextshould return a signal or stay as-is