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

@storybook/theming causes type errors during tsc #7379

Closed
backbone87 opened this issue Jul 11, 2019 · 20 comments
Closed

@storybook/theming causes type errors during tsc #7379

backbone87 opened this issue Jul 11, 2019 · 20 comments

Comments

@backbone87
Copy link
Contributor

backbone87 commented Jul 11, 2019

Describe the bug
Ok this is a fun one. Basically tsc complains about missing react typings, when trying to use create of @storybook/theming.
This is because:

  • @storybook/theming imports @emotion/*
  • which in turn imports react
  • which in turn requires @types/react
  • which would have to be installed manually
  • which would then declare global JSX namespace
  • which would then conflict with the JSX namespace of Vue

To Reproduce

import { create } from '@storybook/theming';

export default create({
  base: 'light',
  brandTitle: 'My Storybook',
});
  • npx tsc

Expected behavior
Use create from @storybook/theming without requiring react typings and therefore no JSX namespace conflicts.

System:

  • OS: any
  • Device: any
  • Browser: any
  • Framework: vue
  • Version: 5

Additional context
Workaround:

  • dont use storybook theming
  • or dont use vue jsx/tsx
  • or run tsc with --skipLibCheck

Solution:

  • provide a create theme function/module that does not use @emotion/*
  • the only stuff that create requires from @emotion/* is import { css, keyframes } from '@emotion/core'; in animation.ts)
  • where css type doesnt require react typings (so no problem here), but is imported from @emotion/core types, which imports react typings for other stuff (this is a problem) -> import css from @emotion/css
  • where keyframes type doesnt require react typings (so no problem here), but its type is declared in @emotion/core type declaration file that imports react typings, but actually dont use react typings for keyframes itself (but for other stuff) -> ask for upstream change in emotion (which would maybe require some reorganisation of types/exports on their part) or reimplement/c&p keyframes into @storybook/theming or dont use keyframes during create
@github-actions
Copy link
Contributor

github-actions bot commented Jul 11, 2019

Automention: Hey @domyen @elevatebart @emilio-martinez @gaetanmaisse @kroeder @pksunkara, you've been tagged! Can you give a hand here?

@ndelangen
Copy link
Member

ndelangen commented Jul 11, 2019

@backbone87 try:

import { create } from '@storybook/theming/create';

Does that solve the issue?

@backbone87
Copy link
Contributor Author

@ndelangen no, see section "solution" in the opening post

@stale stale bot added the inactive label Aug 1, 2019
@backbone87
Copy link
Contributor Author

still relevant

@stale stale bot removed the inactive label Aug 1, 2019
@ndelangen ndelangen added the todo label Aug 1, 2019
@storybookjs storybookjs deleted a comment from stale bot Aug 1, 2019
@selcet
Copy link

selcet commented Oct 29, 2019

Still, very need full custom styling for Storybook.

@vertic4l
Copy link

I'm having the same issue with @storybook/vue... which installs some babel dependency which also changes the typings

@backbone87
Copy link
Contributor Author

Can confirm, this isnt limited to theming anymore. (It has nothing to do with installs, just about what is imported, which means some import during storybook config/bootstrap imports something which imports something etc, until finally react TSX typings get imported, which results in conflict)

the only 2 workarounds left are:

  • --skipLibCheck (not recommended)
  • not use vue jsx/tsx

@ndelangen
Copy link
Member

@backbone87 isn't there some way to make folder specific tsconfig?

@backbone87
Copy link
Contributor Author

not as far as i know. the TSX declarations target the global namespace, so you can never have vue and react TSX shims imported (directly or indirectly) within the same compliation. just importing @storybook/vue will import react TSX d.ts from somewhere. you can prevent TS from checking stuff in node_modules (skipLibCheck), but that may result in incomplete typings.

removing the vue TSX declarations (and not using TSX) is a good workaround, since TSX traction in the vue ecosystem is low, but that may change with vue 3.

@ndelangen
Copy link
Member

@backbone87 I sympathise.

the TSX declarations target the global namespace

That seems like the root of the problem.

I'm guessing that's not configurable either?

@backbone87
Copy link
Contributor Author

At least currently it is not configurable. but i dont think that it is feasible either. it would definitely require changes within typescript itself and maybe even the babel plugin to transpile 2 TSX files differently based on its path. vue tsx gets compiled into vues createElement calls, while react tsx gets compiled to reacts renderComponent (or whatever its called in react).

but i think it is bad for frameworks/libs/packages to (indirectly) import tsx shims in their public typings since the code is compiled already most of the time and therefore the shims arent needed.

when i opened this issue, these conflicts come from emotion packages, which was only imported when using storybook theming, but now the shims get even imported when importing @storybook/vue, but i havent tracked down the import chains, so i cant tell if its still just emotion or other chains as well

@backbone87
Copy link
Contributor Author

fwiw: the real problem seems to be @types/react which should not expose their TSX shims without explicitly importing them -> https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L2939

which means as soon as you want to use react in any way (and even it its just exposing some APIs which have nothing to do with rendering react at all) you get the react shims

@viljark
Copy link

viljark commented Apr 3, 2020

I'm facing the same issue.

Had to add "paths": { "react": [".sink.d.ts"] } and "skipLibCheck": true, to tsconfig.json.

The .sink.d.ts is just an empty file and enables me to use my own namespace JSX for vue, since the react module import eventually will not introduce it's own JSX namespace.

@storybookjs storybookjs deleted a comment from vertic4l Apr 3, 2020
@vertic4l
Copy link

vertic4l commented Apr 4, 2020

Thank you for deleting my comment.
Censorship is what we all need...

@shilman
Copy link
Member

shilman commented May 21, 2020

We've just released zero-config typescript support in 6.0-beta. Please upgrade and test it!

Thanks for your help and support getting this stable for release!

@blake-newman
Copy link
Contributor

@shilman it looks like the @storybook/theming package doesn't cause TS + JSX conflicts.

However it seems the @storybook/knobs package causes this issue still as it ships @types/react-color. I'm unsure if this is intentional.

The above causes the @types/react package to be installed when using the @storybook/knobs package. If the knobs addon can ship without this types dependency then this should resolve the Vue + TSX issues.

@shilman
Copy link
Member

shilman commented May 30, 2020

@blake-newman We’ve just released addon-controls in 6.0-beta!

Controls are portable, auto-generated knobs that are intended to replace addon-knobs long term.

Can also try to fix the knobs issue but if you’re able to upgrade i recommend it!

@yandongxu
Copy link

Still have problem with JSX type conflicts, when i write stories like this:

import { Story } from '@storybook/vue';

const Template: Story = (args, { argTypes }) => ({
  components: { Card },
  props: Object.keys(argTypes),
  template: '<Card v-bind="$props" />',
});

export const Default = Template.bind({});
Default.args = {
  title: 'MyTitle',
};

@storybook/vue imported react JSX types

@akay64
Copy link

akay64 commented Mar 4, 2022

Currently facing the same issue, one solution I found is to not use TypeScript at all for writing stories. Story files will all be ".js" files.

@ndelangen
Copy link
Member

This is resolved in storybook 7.0 beta because we bundle in emotion.

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

9 participants