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 vue type checking not working #14987

Closed
fallemand opened this issue May 19, 2021 · 8 comments
Closed

Storybook vue type checking not working #14987

fallemand opened this issue May 19, 2021 · 8 comments

Comments

@fallemand
Copy link

fallemand commented May 19, 2021

Describe the bug
After enabling type check in vue, I would expect to see the errors when building, but nothing happens.

To Reproduce
1- Add typescript check:

// main.ts
module.exports = {
  ...,
  typescript: {
    check: true,
  },
}

2- Add any type error in a vue file

const test: string = 3;

3- Run npm run storybook
It doesn't complain.

System

  System:
    OS: macOS 11.2.3
    CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
  Binaries:
    Node: 14.15.1 - ~/.nvm/versions/node/v14.15.1/bin/node
    npm: 7.12.1 - ~/.nvm/versions/node/v14.15.1/bin/npm
  Browsers:
    Chrome: 90.0.4430.212
    Firefox: 88.0
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-a11y: ^6.2.7 => 6.2.9 
    @storybook/addon-essentials: ^6.2.7 => 6.2.9 
    @storybook/vue: 6.2.7 => 6.2.9 
@shilman
Copy link
Member

shilman commented May 21, 2021

Is that type error in a .vue file or a .ts/.tsx file?

@fallemand
Copy link
Author

@shilman On a vue file.

@shilman
Copy link
Member

shilman commented May 21, 2021

I think the built-in type checking only works on .ts/.tsx files (not entirely sure)

@fallemand
Copy link
Author

@shilman Adding the issue on a ts file, doesn't produce the error either. I don't see fork-ts-checker-webpack-plugin being executed.

@pocka
Copy link
Contributor

pocka commented May 29, 2021

Investigated, then found the root cause. However, I'm not sure which approach should I take to fix it.
Any ideas?

The problem

@storybook/vue (Vue2) ignores fork-ts-checker-webpack-plugin as stated here. However, vue preset always set transpileOnly: true so neither ts-loader's type-checking process nor fork-ts-checker-webpack-plugin kicks in.

Possible fixes

A) Change transpileOnly based on typescript.check

Read typescriptOptions.check in here then set transpileOnly based on that value. IMO this is the most straightforward approach, but I'm not sure it's okay to do below to access typescriptOptions in a framework preset file (framework-preset-vue.ts).

  const typescriptOptions = await presets.apply<TypescriptConfig>('typescript', {} as any);

B) Use the base fork-ts-checker-webpack-plugin like other frameworks

Remove vue from this. In addition to that, we should add extensions.vue: true on here. (also for webpack5 builder)

This leaks framework-specific configuration to builder iframe-webpack.config.ts. Also, we'll need to update docs.

@shilman
Copy link
Member

shilman commented May 29, 2021

I can't remember the full rationale for why Vue needed its own Typescript setup, but I'm pretty sure that it does. I think @graup implemented this, so perhaps he can chime in.

@pocka do you think you can make a PR with option A? It sounds reasonable to me!

@graup
Copy link
Contributor

graup commented May 29, 2021

I don't remember 100%, I believe we had some offline discussions involving @mrmckeb and this PR, after which we concluded that Vue just works better with ts-loader.

Option A sounds good to me too. I'm not sure why we didn't include that back when we added zero-config ts. I believe at that time it was just assumed that most people don't want type-checking in their storybook builds due to the performance overhead it introduces. That typescript.check option was introduced later but probably just forgotten to be added to the vue preset.

pocka added a commit that referenced this issue May 29, 2021
#14987

TypeScript checks in `@storybook/vue` has not been enabled. This commit
enables users to turn on type checking process powered by ts-loader.

From performance perspective, fork-ts-checker-webpack-plugin is better
especially for large projects. However, it needs one of 1) modifying
type-checker options in each builders or 2) adding
fork-ts-checker-webpack-plugin as a dependency of `@storybook/vue`.
I think both comes with huge downsides and it should be done in user
side by setting `typescript.check: false` and configuring the plugin
manually.
@shilman
Copy link
Member

shilman commented May 30, 2021

Yay!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.0-beta.5 containing PR #15089 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.

@shilman shilman closed this as completed May 30, 2021
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

4 participants