Skip to content
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

Array of elements returned from storyFn not allowed in v5.2 #8116

Closed
vhenzl opened this issue Sep 18, 2019 · 8 comments
Closed

Array of elements returned from storyFn not allowed in v5.2 #8116

vhenzl opened this issue Sep 18, 2019 · 8 comments

Comments

@vhenzl
Copy link

vhenzl commented Sep 18, 2019

Describe the bug
This is more a question. And possibly a bug report for TS typing.

Before storybook 5.2, it was possible to return an array of elements from storyFn like this:

const colors = ['red', 'green', 'blue'];

storiesOf('Button', module)
  .add('With color', () => colors.map(color => <Button color={color} />));

After upgrade to 5.2 the code still works but TypeScript reports following error:

Type 'Element[]' is missing the following properties from type 'ReactElement<unknown, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<any, any, any>)>': type, props, key

In some other cases the error is different (but essentialy the same):

Type 'Element[]' is not assignable to type 'ReactElement<unknown, string | ((props: any) => ReactElement<any, string | ... | (new (props: any) => Component<any, any, any>)> | null) | (new (props: any) => Component<any, any, any>)>'.

The question: Can storyFn return an array of elements? Or it has never been meant to be used like that?

Expected behavior
TS typing for storyFn allows an array of elements as its return type.

System:
OS: Windows 10
Node: 10.15.3 - C:\Program Files\nodejs\node.EXE
Yarn: 1.17.3 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD
@storybook/react: ^5.2.1 => 5.2.1
TypeScript: 3.6.3

@shilman
Copy link
Member

shilman commented Sep 19, 2019

@nielsvanmidden
Copy link

@vhenzl Does it help to wrap your array with React.Fragment?
Accordingly, I have an issue with having a function that returns null. TypeScript complains with the same message.

This is desired when using decorators:

const enhancerModal = (
  modalType: ValueOf<ModalContentKeys>,
  timeout = false,
  response?: GiftCardOverview,
) => store => {
  services.giftCards = new (class extends GiftCardServiceMock {
    public async fetchGiftCards() {
      if (response) return mockResponseSuccess(response, { timeout });
      throw BadRequestError.somethingWentWrong();
    }
  })();
  store.dispatch(openModalWindow(modalType));
};

const enhancerClaimables = (response?: GiftCardOverview, timeout?: boolean) =>
  enhancerModal(MODAL_CONTENT_KEYS.ADD_CLAIMABLES, timeout, response);

storiesOf(STORY_NAME, module)
  .addDecorator(initDecoratorsWithDataSet(defaultCheckoutDataSet, enhancerClaimables(giftCardCheckoutResponse)))
  .add('default', () => null);

I'm not so much interested in rendering a component here directly. But rather in a rendered component based on our Redux application state. In this case a modal opens once fetching data succeeded.

@vhenzl
Copy link
Author

vhenzl commented Sep 19, 2019

@vhenzl Does it help to wrap your array with React.Fragment? ...

@nielsvanmidden Yes, that’s what I did to get rid of the errors. I wrapped body of storyFn with (<>{ and }</>). Doing so the function returns single element and compiler is happy.

Sent with GitHawk

@emilio-martinez
Copy link
Contributor

The question: Can storyFn return an array of elements? Or it has never been meant to be used like that?

@vhenzl Each Storybook framework adapter is a bit different in what they allow as valid return from the render function, but I believe they're all roughly aligned in that they accept a single element rather than an array of elements to render. There's practical reasons for it in some packages such as performing diffing to determine when it's appropriate to re-render the story, for example.

@vhenzl
Copy link
Author

vhenzl commented Sep 21, 2019

@emilio-martinez OK, makes sense. Thanks for the clarification.

Sent with GitHawk

@ndelangen
Copy link
Member

Does it work at runtime?

if so, we should fix the types.
if not, I'm hesitant to add the feature

My guess is, it does work, it's just the typings that are wrong. We'd love a PR fixing these types. Would you be able to contributing @vhenzl ?

@vhenzl
Copy link
Author

vhenzl commented Sep 25, 2019

@ndelangen Yes, it works. It always has, at least since Storybook v4. I believe it's a typing problem related to #7054.

Unfortunately, no, I don't think I'm able to contribute in this part.

@shilman
Copy link
Member

shilman commented Oct 7, 2019

ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.12 containing PR #8197 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

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

@shilman shilman closed this as completed Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants