Storybook controls shouldn't pass 'undefined' by default

[DaniGuardiola]

Now that storybook doesn't infer defaults for controls since 6.3+, there's an unexpected behavior where stories will get undefined explicitly as control values. This can mess up some components (in this case, React), because { someProp: undefined } is different than {} and it will result in strange behavior. For example, if you're handling defaults by doing something like { ...DEFAULT_PROPS, props }, then a value of undefined will overwrite the default.

See discussion in issue #15378 for more details. Seems like this was introduced in #14579.

Mentioning people involved / with the same issue: tmeasday shilman penx mboettcher

[pupudu]

I faced this issue today, and may I take the liberty of adding one more scenario to the issue.

In addition to receiving an object with undefined values, this object contains keys of disabled controls as well. For example, I have this entry in a story:

Default.argTypes = {
  Components: { table: { disable: true } }, // i.e. disable controls for the prop "Components"
};

And when the story renders, the props coming from the control args take the following form

{
  Components: undefined, // prop correspnding to the disabled control
  interval: undefined, // other props
  value: undefined, // other props
}

Thanks a lot for reporting this issue, and thanks to the storybook team for this amazing product.

[tmeasday]

@pupudu I suppose the disabled case isn't all that different as from the core's perspective there should be no difference (disabling it just means show no UI to change/set it).

[shilman]

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

npx sb upgrade --prerelease

[shilman]

Crikey!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.5 containing PR #15654 that references this issue. Upgrade today to the @latest NPM tag to try it out!

npx sb upgrade

[DaniGuardiola]

@shilman I'm still running into this issue on this version. It seems to be better now but it still passes undefined to some props (sometimes only after I load the story for the second time). The props that are being set to undefined are the ones for which the controls UI is loaded (e.g. not the ones with "Set boolean" or "-"). I believe before this fix, all props received undefined.

Btw, here's a workaround I've come up with, with (wonky but functional) typescript support, in case it's useful to anyone:

type MergeDefaultProps<D, P, M extends D | P = D | P> = {
  [K in keyof D | keyof P]: K extends keyof M
    ? K extends keyof D
      ? Exclude<M[K], undefined>
      : M[K]
    : K extends keyof P
    ? P[K]
    : K extends keyof D
    ? D[K]
    : never;
};

function usePropsWithDefaults<P, D>(
  props: P,
  defaults: D
): MergeDefaultProps<D, P> {
  const propsCopy = { ...props };
  Object.entries(props).forEach(([key, value]) => {
    if (value === undefined) delete propsCopy[key];
  });
  return { ...defaults, ...propsCopy } as unknown as MergeDefaultProps<D, P>;
}

const DEFAULT_PROPS = {
  example: "value"
} as const;

function Component(
  props: ComponentProps
): ReactElement {
  // resolved props
  const p = usePropsWithDefaults(props, DEFAULT_PROPS);
  
  // etc...
}

[tmeasday]

@DaniGuardiola do you have a reproduction of the problem we can look at?

[revik]

@tmeasday @shilman same here - still facing the same problem as @DaniGuardiola .
Here's a sample in codesandbox
I'd expect the controls to show isDisabled control to be set with false and the type to be set with "button".

Am I missing something here?

[tmeasday]

@revik -- I think you are getting two things confused.

This ticket is about the component or story template being passed undefined in a control value, rather than no value at all. I don't believe that is the case in your example.

I think what you refer to is the change in 6.3 to no longer set an arg to the (inferred) component's default value, and thus show it in the control: #15378. As you can see by that ticket, it was an intentional change and although perhaps what we display in the UI for such unspecified args could be improved, we are not going to revert to the behaviour of attempting (and sometimes failing) to select the correct value.

[revik]

@tmeasday I've modified the example to explicitly use a decorator to set the args using the component's defaultProps.
I also see the args set correctly with the desired values, but the controls do not reflect it.

[tmeasday]

@revik I am not seeing that change?

[revik]

I mean the changes here - https://codesandbox.io/s/wonderful-panini-fqph0?file=/.storybook/preview.js
But as I understand now - it doesn't matter what I pass in that matter.
The only way to set the controls' values is to set the args property on the exported Story or the default exported story configuration's args property.
I can't do it using a decorator, right?

[revik]

@tmeasday - Just one more question - as we are using Storybook for automation tests - we were previously able to select the html element of the control of a specific prop an id with the value of the prop name (ie: document.querySelector('#propName'), but now that the control have been hidden with the 'Set ' button ('Set boolean' etc) when the prop has no value - there is no obvious selector set there. Is there a thought to set an id there (maybe 'set-propName' or 'placeholder-propName'?

[tmeasday]

I can't do it using a decorator, right?

No, you can't that's right. I think a work around here would be to set the args at the component level using the default props:

export default {
  component: Button,
  args: Button.defaultProps,
};

prop has no value - there is no obvious selector set there. Is there a thought to set an id there (maybe 'set-propName' or 'placeholder-propName'?

Can you make an issue for this? This seems like a reasonable feature request.

[revik]

Thanks!
Opened issue - #15690