Controls: Rework default value handling

[shilman]

Instead of initializing an optional control value to its default value, initialize it to undefined

Rationale

Default value handling is currently problematic in Storybook's Arg handling.

We statically analyze the code to determine the default value. This is impossible to do statically in all cases, and on top of that there are various inconsistencies between how different libraries handle the output (e.g. string literal vs expression).

This leads to issues such as #14387 #14370 #13919 and many more.

However, since the default value gets set automatically when NO value is passes to the component, passing undefined is a much better option. It sidesteps all the existing issues and the component will use the runtime default value, which will be correct.

What's needed

A few different things are needed to make this happen:

• All controls must be able to handle undefined and reset
• We need an ArgEnhancer construct that mirrors the ArgTypeEnhancer so that addons can add values
• addon-actions needs to move from ArgTypeEnhancer to ArgEnhancer
• The control logic must be updated to only initialize using args, not argType.defaultValue
• We probably need a migration/deprecation strategy since technically this is a breaking change
• Delete all the old code

Open question:

• Do we want argTypes.x.defaultValue to set args.x automatically? => YES

[ThibaudAV]

a thought that comes to me while reading : 😁
not sure if I understand the initialize it to undefined
but the case of args:{} is not the same as {foo: undefined}
So if I don't have a default value defined on the foo property, when I click on reset I expect to find {}.

[kylegach]

Can anyone think of a workaround that's better than applying a decorator like so:

(Story, { args }) => {
  const { argName: ignoreArgName, ...restArgs } = args;
  return <Story args={restArgs} />;
},

This approach has two major limitations:

1. argName no longer has an effect on the story, so this only works for args that have disabled controls or table rows
2. You have to manually ignore each individual arg

[shilman]

@kylegach @ThibaudAV we're introducing a new ArgType.disable property that will tentatively do this for you. PR is here, but I haven't added the "removing from the args object" part yet #13890

UPDATE: added

[ThibaudAV]

@shilman Okay
We could already not add all the properties in agrTypes but this has a major impact:

• the documentation for private properties or internal methods would not be generated anymore

We can't have documentation on a class without assigning a value for each property in args.
bad combo with compodoc -> argTypes -> args + angular renderer ^^

I think that with the rework it could be nice to differentiate agrTypes for docs only from those with an interaction adding a value in args 🤷‍♂️

UPDATE ⬆️ I need to test 😄

[shilman]

@ThibaudAV great point 🤦

i'll sleep on it and see if i can come up with something better

[shilman]

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.0-alpha.31 containing PR #14901 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[mboettcher]

Not sure if I'm missing something here, but I have issues with initializing all props to undefined, since {foo: undefined} is not the same as {}. For example I use ant-design TimePicker (https://ant.design/components/time-picker/). Setting suffixIcon to undefined will lead to no icon rendered instead of rendering the default icon.

I think there should be a difference between setting a prop to undefined on purpose and not setting a prop (then the key should not exist instead of being set to undefined).

[shilman]

@mboettcher it's being discussed here FYI #15378