-
-
Notifications
You must be signed in to change notification settings - Fork 392
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
docs(core): reusable fields & reusable field utility type #1084
Conversation
View your CI Pipeline Execution ↗ for commit f0df804.
☁️ Nx Cloud last updated this comment at |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1084 +/- ##
==========================================
+ Coverage 86.61% 86.75% +0.13%
==========================================
Files 29 29
Lines 1158 1155 -3
Branches 301 298 -3
==========================================
- Hits 1003 1002 -1
+ Misses 142 140 -2
Partials 13 13 ☔ View full report in Codecov by Sentry. |
7b64244
to
4256a04
Compare
92b67c7
to
616d520
Compare
Sorry for the push forces... was looking at the incorrect section of code and wondering why it wasn't changing. Friday’s for you 🙃 |
616d520
to
bb83da8
Compare
/** | ||
* The return type use useForm with pre-populated generics | ||
*/ | ||
export type UseFormReturnType<TForm, TFormValidator> = TFormValidator extends |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to get rid of the TFormValidator slot, since TFormValidator is just Validator<TForm, unknown> | undefined. But so far my efforts have been fruitless.
{({ handleChange, handleBlur, state }) => ( | ||
<input | ||
value={state.value} | ||
onChange={(e) => handleChange(e.target.value as any)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another improvement would be to get Ts to infer the handleChange value so the update is type safe.
524d024
to
8a4523a
Compare
Might be a dup with #825 ? |
@Talent30 yeah, I was talking with crutchcorn about this on Friday and he made me aware of it. He said to leave it open for now, and he'll link it to his issue. Though, I'm not sure how useful it'll be, as after reading though the linked Pr, they've run into all the same problems I have. 🙃 |
wonder if it'd be possible to do something like
then to use it it's just like
|
@bzbetty I think the issue with the proposed solution is that if the form is apart of the component then your re-instantiating the form on every component. So instead of having one form controlling the fields, you will have multiple standalone forms controlling a single field then passing the data back to whatever the args is. After seeing the proposed solutions in #825, and after banging my head against the keyboard for a couple weeks, I think its probably the best way to go. Though I will admit if you could get the form typing your solution would probably address the internal typing issues I ran into in my attempt. |
Closing in favor of an alternative API we're proposing here: #1179 |
A continue on from #636, this PR introduces docs for reusable components/fields as well as introduces a Ts utility type for inferring valid field names, for the relevant component.
Much like my last PR this is something I was trying to do for a work project, but not having any docs or examples hindered the process, so I figured I'd give it a go.
Guidance wanted:
Specifically with this line
onChange={(e) => handleChange(e.target.value as any)}
(line 28 in reusable-fields) I can't stand the casting of any, however I can figure a way for Ts to infer this value. Any suggestions is greatly appreciated!Sandbox
I have a code sandbox for playing around with this here, you'll see theres a folder called solutions that has two files:
The rest of the files can be ignored but if you want to see how I got here they kind of explain my reasoning.