Add optional typing to the useArgs hook #18715
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit e566adb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
Hey @Gabriel5934 thanks for your contribution! I wonder if this contribution is similar to #18122. cc @shilman |
| @@ -442,7 +442,11 @@ export function useAddonState<S>(addonId: string, defaultState?: S) { | |||
| return useSharedState<S>(addonId, defaultState); | |||
| } | |||
|
|
|||
| export function useArgs(): [Args, (newArgs: Args) => void, (argNames?: string[]) => void] { | |||
| export function useArgs<Args extends Record<string, any> = { [key: string]: any }>(): [ | |||
There was a problem hiding this comment.
| export function useArgs<Args extends Record<string, any> = { [key: string]: any }>(): [ | |
| export function useArgs<TArgs extends Args = Args>(): [ |
There was a problem hiding this comment.
We should have an Args type that we can use instead of Record<string, any>
There was a problem hiding this comment.
@Gabriel5934 Thanks so much for your contribution and patience on this one. We just got around to looking at it as a group and here's the collective wisdom on this one:
This is the useArgs that is exposed in the manager (Storybook's UI), not the preview, so the type will change for every single components' story. Therefore making the type generic might not be so useful? There is another PR that does the same for the preview (Storybook's canvas for displaying user components), and perhaps that was what you were intending?
|
@kasperpeulen you're working on an huge effort to improve the situation, and that includes what this PR is doing, correct? WDYT we should do with this PR? |
| @@ -442,7 +442,11 @@ export function useAddonState<S>(addonId: string, defaultState?: S) { | |||
| return useSharedState<S>(addonId, defaultState); | |||
| } | |||
|
|
|||
| export function useArgs(): [Args, (newArgs: Args) => void, (argNames?: string[]) => void] { | |||
| export function useArgs<Args extends Record<string, any> = { [key: string]: any }>(): [ | |||
There was a problem hiding this comment.
Ow I now just read @shilman 's comment and looked at the changed code.
I'm afraid we can't do this. @Gabriel5934
The reason is that it would make the code less sound.. Though it's nice to get auto-completion, it's really dangerous to tell yourself you'll receive args of type X, but then at runtime you get a completely different object.
Now typescript tells you everything is fine, but in actuality it will likely throw at runtime.
And in this case when the code throws... it's a fatal problem for the storybook manager code.
We really appreciate your time, energy and effort that you put into this PR, but unfortunately we can't merge it :(
I hope you understand our reasoning. ❤️
Issue:
argsreturned by theuseArgshook cannot be typed.What I did
Added a generic type to the
useArgsfunction so the returnedargsobject type can match the actual component props type.Also added default value to the generic of
{[key]: string}so the hook still works as intended without passing a generic touseArgs.