Feature: Add defaultProps helper function #1794
Conversation
|
|
Ok. This is interesting. I can see value in special casing it for types. I generally don't like increasing the API surface but I have a suspicion this is the 90% case so that seems like a good add. I'd love to get some more feedback on this feature, but definitely something I think we'd consider. |
|
I believe we should try to solve this within the type system before admitting defeat and resorting to extend the API, e.g. using a switch generic like |
|
I can see enough reason for this to be included, albeit maybe with some iteration of its design. As I see it, With Personally I would reverse the parameters (props first, then defaults), so that the default values follow their "declaration" as they do with default parameters, and also exclude props which are required since providing a default for those props is meaningless. |
I agree with this. I've been using this as a helper function quite extensively in my current project and having the signature be I implemented it this way originally to mirror the current export type SliderProps = {
label?: string;
value?: number;
onChange?: (value: number) => void;
min?: number;
max?: number;
step?: number;
origin?: "start" | "center";
class?: string;
};
const Slider: Component<SliderProps> = (unresolvedProps) => {
const props = defaultProps(unresolvedProps, {
label: "",
min: 0,
max: 100,
step: 1,
origin: "start",
class: "",
});
// blah...
};EDIT: I went ahead and pushed a commit to swap the argument order. But this can be reverted if opinions differ. |
I see what you're saying but if I can add my 2 cents about this: I think That's just my thoughts. I definitely understand the concerns with extending the API though. |
| export type DefaultProps<T, K extends keyof T> = MergeProps<[Required<Pick<T, K>>, T]>; | ||
| export function defaultProps<T, K extends keyof T>( | ||
| props: T, | ||
| defaults: Required<Pick<T, K>> | ||
| ): DefaultProps<T, K> { | ||
| return mergeProps(defaults, props); | ||
| } |
There was a problem hiding this comment.
type DefaultableProps<T, K extends keyof T> = {
[P in K]-?: undefined extends T[P] ? Exclude<T[P], undefined> : never;
}- Required<Pick<T, K>>
+ DefaultableProps<T, K>I haven't fully tested this but this should allow only defaults that aren't already necessary in props.
|
Hey there, just tried out the type Props = {
initialCount: number;
firstName?: string;
middleName?: string | undefined;
lastName?: string;
notDeclared?: string
};
const $someNumber = 3;
export const myDefaultProps = {
firstName: $someNumber >= 3 ? "John" : undefined,
middleName: "Ron",
lastName: $someNumber > 3 ? "Doe" : undefined,
} as const satisfies Partial<Props>;
const props = defaultProps(props, myDefaultProps);So given this block here 👆, this should result in something like this, no?
In this example |
|
Worked the bit out with where only keys that are actually in the props should be allowed in the defaults I think, like @otonashixav mentioned. Check out the new playground Note A thought: is Tip You now get proper TS suggestions with this (maybe I also missed something, but didn't work for me somehow) |
Summary
mergeProps provides the current standard way of adding default props to a component, but it has some weaknesses when it comes to typing (see #1526). defaultProps essentially adds a subset of mergeProps functionality but with more restricted typing for the specific case of providing default props to a component. It has two main advantages:
"start" | "center" | "end"does not get transformed intostring)Even though technically the functionality of defaultProps can be achieved with mergeProps, defaultProps provides a much nicer typescript experience for an extremely common use case.
How did you test this change?
packages/solid/test/component.spec.tspackages/solid/test/component.type-tests.tspnpm test