-
Notifications
You must be signed in to change notification settings - Fork 994
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
@redwoodjs/forms: Convert to TS #1431
Conversation
07e414c
to
50cdd57
Compare
ded0faf
to
8c3eb2d
Compare
@peterp Ready for review! |
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'm not a TS expert, by any measure, I'm still trying to get my head around conditional generics, but I've made two suggestions.
setCoercions: React.Dispatch<React.SetStateAction<Coercions>> | ||
} | ||
|
||
const CoercionContext = React.createContext({} as CoercionContextInterface) |
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.
Since we don't have default values for this context, we can do something like this:
React.createContext<Partial<CoercionContext>>({});
https://fettblog.eu/typescript-react/context/#context-without-default-values
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 don't want to do this. It's effectively make all attributes of the context optional, which I don't want. We actually do have kind of "default values". They're just not set here. They're set further down as props to the Provider. Whatever you put here is never going to be used. It's only for when you use the context outside of its Provider, which we never do.
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.
That's fair. We're not trying to satisfy TS here... but rather our intention.
@Tobbe This is great, thanks! |
I have some TODOs left in the code that I need to clear up