storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
83.42k stars 9.12k forks source link

Upgrade to Storybook@6.5 does not work without vite builder #18286

Closed drik98 closed 1 year ago

drik98 commented 2 years ago

Describe the bug I am currently on version 6.4.22 of storybook. When trying to run npx sb@latest upgrade it tries to upgrade as usual but it suddenly stops without any further message at the step ๐Ÿ”Ž checking 'builder-vite'.

Note that I do not use the vite builder. Is this a breaking change? Do we have to move to vite in order to use newer versions of storybook?

To Reproduce Please create a reproduction by running npx sb@next repro and following the instructions. Read our documentation to learn more about creating reproductions. Paste your repository and deployed reproduction here. We prioritize issues with reproductions over those without.

Currently no time for that. Sorry.

System Please paste the results of npx sb@next info here.

Environment Info:

  System:
    OS: macOS 12.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 12.18.3 - ~/Library/Caches/fnm_multishells/57824_1652876435650/bin/node
    npm: 6.14.6 - ~/Library/Caches/fnm_multishells/57824_1652876435650/bin/npm
  Browsers:
    Chrome: 101.0.4951.64
    Firefox: 100.0
    Safari: 15.4
  npmPackages:
    @storybook/addon-actions: ^6.4.22 => 6.4.22 
    @storybook/addon-essentials: ^6.4.22 => 6.4.22 
    @storybook/addon-links: ^6.4.22 => 6.4.22 
    @storybook/vue: ^6.4.22 => 6.4.22 

Additional context Here is the complete log:

npx sb@latest upgrade                                                         
 โ€ข Checking for latest versions of '@storybook/*' packagesinfo ,,npx: Installierte 267 in 3.544s
info Unexpected token '.'
info 
 โ€ข Installing upgrades โ€ข Preparing to install dependencies. โœ“

> <MY_PROJECT@version> prepare /Users/drik98/<PROJECT_PATH>
> rimraf build lib && node src/build/compile-vue-components src/js build && babel --copy-files build --out-dir lib && node src/license/crawler

< MOVING SOME src/**/*.vue FILES TO build/**/*.js >

Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Successfully compiled 52 files with Babel (1671ms).
npm WARN @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining@7.17.12 requires a peer of @babel/core@^7.13.0 but none is installed. You must install peer dependencies yourself.
npm WARN @babel/plugin-proposal-class-static-block@7.17.12 requires a peer of @babel/core@^7.12.0 but none is installed. You must install peer dependencies yourself.
npm WARN @vue/component-compiler@4.2.4 requires a peer of postcss@>=6.0 but none is installed. You must install peer dependencies yourself.
npm WARN eslint-plugin-vue@6.2.2 requires a peer of eslint@^5.0.0 || ^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN fork-ts-checker-webpack-plugin@6.5.2 requires a peer of typescript@>= 2.7 but none is installed. You must install peer dependencies yourself.
npm WARN postcss-loader@4.3.0 requires a peer of postcss@^7.0.0 || ^8.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN prosemirror-utils@0.9.6 requires a peer of prosemirror-tables@^0.9.1 but none is installed. You must install peer dependencies yourself.
npm WARN ts-loader@8.4.0 requires a peer of typescript@* but none is installed. You must install peer dependencies yourself.

up to date in 12.406s

258 packages are looking for funding
  run `npm fund` for details

. โœ“
๐Ÿ”Ž checking 'cra5'
๐Ÿ”Ž checking 'webpack5'
๐Ÿ”Ž checking 'angular12'
๐Ÿ”Ž checking 'mainjsFramework'
๐Ÿ”Ž checking 'eslintPlugin'
โ•ญโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ
โ”‚                                                                                                                                      โ”‚
โ”‚   We've detected you are not using our eslint-plugin.                                                                                โ”‚
โ”‚                                                                                                                                      โ”‚
โ”‚   In order to have the best experience with Storybook and follow best practices, we advise you to install eslint-plugin-storybook.   โ”‚
โ”‚                                                                                                                                      โ”‚
โ”‚   More info: https://github.com/storybookjs/eslint-plugin-storybook#readme                                                           โ”‚
โ”‚                                                                                                                                      โ”‚
โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ
โœ” Do you want to run the 'eslintPlugin' fix on your project? โ€ฆ yes
โœ… Adding dependencies: eslint-plugin-storybook
npm WARN notsup Unsupported engine for @typescript-eslint/experimental-utils@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/experimental-utils@5.25.0
npm WARN notsup Unsupported engine for @typescript-eslint/utils@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/utils@5.25.0
npm WARN notsup Unsupported engine for @typescript-eslint/types@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/types@5.25.0
npm WARN notsup Unsupported engine for @typescript-eslint/scope-manager@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/scope-manager@5.25.0
npm WARN notsup Unsupported engine for @typescript-eslint/typescript-estree@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/typescript-estree@5.25.0
npm WARN notsup Unsupported engine for @typescript-eslint/visitor-keys@5.25.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: @typescript-eslint/visitor-keys@5.25.0
npm WARN notsup Unsupported engine for eslint-visitor-keys@3.3.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"12.18.3","npm":"6.14.6"})
npm WARN notsup Not compatible with your version of node/npm: eslint-visitor-keys@3.3.0
npm WARN @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining@7.17.12 requires a peer of @babel/core@^7.13.0 but none is installed. You must install peer dependencies yourself.
npm WARN @babel/plugin-proposal-class-static-block@7.17.12 requires a peer of @babel/core@^7.12.0 but none is installed. You must install peer dependencies yourself.
npm WARN @vue/component-compiler@4.2.4 requires a peer of postcss@>=6.0 but none is installed. You must install peer dependencies yourself.
npm WARN eslint-plugin-vue@6.2.2 requires a peer of eslint@^5.0.0 || ^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN fork-ts-checker-webpack-plugin@6.5.2 requires a peer of typescript@>= 2.7 but none is installed. You must install peer dependencies yourself.
npm WARN postcss-loader@4.3.0 requires a peer of postcss@^7.0.0 || ^8.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN prosemirror-utils@0.9.6 requires a peer of prosemirror-tables@^0.9.1 but none is installed. You must install peer dependencies yourself.
npm WARN ts-loader@8.4.0 requires a peer of typescript@* but none is installed. You must install peer dependencies yourself.
npm WARN tsutils@3.21.0 requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.

+ eslint-plugin-storybook@0.5.12
added 34 packages from 43 contributors in 25.508s

265 packages are looking for funding
  run `npm fund` for details

โœ… Configuring eslint rules in .eslintrc.js
โœ… Adding Storybook to extends list
โœ… fixed eslintPlugin
๐Ÿ”Ž checking 'builder-vite'
shilman commented 2 years ago

@drik98 i'm confused. is something actually broken?

what's going on is that we introduced a feature in 6.4 called automigrations. when you upgrade, it checks for some common configuration changes that needs to be fixed. if it finds something, it will ask you if you want to make the change. if it doesn't prompt you, it means that it didn't find anything that needs changing.

drik98 commented 2 years ago

It just stops with this step. No prompt or anything. No error message. No cvs changes, everything gets reset.

Note that npx storybook init fails as well at the same point if I remove everything storybook related. ( Just a few days ago I was still working with 6.4.22)

shilman commented 2 years ago

@drik98 I see, thanks for the clarification. Do you a have a reproduction repo you can share? If not, can you create one? See how to create a repro.

Alternatively, perhaps you could go into node_modules/@storybook/cli/dist/cjs/automigrate/fixes/builder-vite.js and add some console.log settings to see exactly where it's hanging?

drik98 commented 2 years ago

Hello @shilman,

I am not able to create a repro as it instantly works when selecting vue2 as a preset.

But I was just able to to debug the builder-vite.js file as you said. run and prompt are never executed.

The following happens in check:


const builder = main.getFieldValue(['core', 'builder']);
// builder --> undefined

const builderName = typeof builder === 'string' ? builder : builder === null || builder === void 0 ? void 0 : builder.name;
// builderName --> undefined

if (builderName !== 'storybook-builder-vite') {
  return null;
}
// as builderName is undefined" this evaluates to true and the function returns null. That seems to be where the upgrade process just stops.
drik98 commented 2 years ago

Upon further investigation it seems in node_modules/@storybook/cli/dist/cjs/automigrate/index.js it just skips the check so that should not be why the upgrade process just stops.

Just found out about the --skip-check flag. The issue seems to be unrelated to vite or any checks. It just does not want to upgrade the storybook version it seems.

yannbf commented 2 years ago

@IanVS is this related to the vite automigration?

shilman commented 2 years ago

@yannbf @drik98 I think it's finished. It loops through all the automigrations and builderVite is the last automigration. The code @drik98 has highlighted above returns null, meaning that there is nothing left to do, which is consistent with the fact that the project is not using vite.

I guess the question is why doesn't the process exit after that?

@yannbf I know you were going to change the logging behavior of automigrate. Perhaps you could add a โœ… done at the end?

drik98 commented 2 years ago

Yeah, a "Done" would probably be a good addition even though there seems to be nothing wrong with automigrate (or vite, I think).

The problem seems to lie somewhere else. When checking the code of the upgrade script the "automigrate" step seems to be the last IIRC. Still none of the @storybook/* dependencies seem to have been upgraded at this point (or later).

I am just so confused as to why nothing happens with no error or termination message.

drik98 commented 2 years ago

Where and when exactly do the @storybook/* dependencies get updated? If you point me to the correct file maybe I can debug it and find the error.

yannbf commented 2 years ago

Where and when exactly do the @storybook/* dependencies get updated? If you point me to the correct file maybe I can debug it and find the error.

I really appreciate your help!

https://github.com/storybookjs/storybook/blob/4b849f9bcd1d76bd6a016d50f7f1cb8f7809ed31/lib/cli/src/upgrade.ts#L166

drik98 commented 2 years ago

No need to thank me. I greatly appreciate your work with storybook. It really makes developing and documenting much easier!

So I managed to install version 6.5.4 and successfully build it. I could not do it with the "cli" but I think the problem lies with my machine.. What I found out is the command you linked above evaluates to

npx npm-check-updates@latest /storybook/ --upgrade --target latest

because flags contains [ '--upgrade', '--target', 'latest' ].

Now the ncu command logs the following:

,,npx: Installierte 259 in 6.658s
Unexpected token '.'

As you can maybe see above (issue description )that already happened from the beginning. But It also happens when I run the command on its own. So the problem does not lie with storybook but with either ncu or me.

I did some googling and it seems to be a problem with npm itself (or perhaps in combination with fnm/nvm).

I installed npm-check-updates globally using yarn (normally not using yarn) and executed the command without npx

npm-check-updates /storybook/ --upgrade --target latest

and voila it works.

drik98 commented 2 years ago

The weird part for me is that npm-check-updates apparently encounters some kind of error ("Unexpected token '.'") but doesn't seem to throw an error after it, which then results in Storybook not being able to throw an error.

shilman commented 1 year ago

Weโ€™re cleaning house! Storybook has changed a lot since this issue was created and we donโ€™t know if itโ€™s still valid. Please open a new issue referencing this one if: