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

Webpack 5 upgrade - ts-loader module build fails on storybook run #14170

Closed
marjisound opened this issue Mar 8, 2021 · 8 comments
Closed

Webpack 5 upgrade - ts-loader module build fails on storybook run #14170

marjisound opened this issue Mar 8, 2021 · 8 comments

Comments

@marjisound
Copy link

marjisound commented Mar 8, 2021

Describe the bug
Hi there, after upgrading webpack version to 5, running build-storybook or start-storybook is causing the ts-loader module build to fail. TypeError: Cannot read property 'tap' of undefined

WARN ./src/components/anchor.stories.tsx WARN Module build failed (from ./node_modules/ts-loader/index.js): WARN TypeError: Cannot read property 'tap' of undefined WARN at makeAssetsCallback (/Users/marjank/code/apps-rendering/node_modules/ts-loader/dist/instances.js:218:50) WARN at addAssetHooks (/Users/marjank/code/apps-rendering/node_modules/ts-loader/dist/instances.js:224:9) WARN at Object.initializeInstance (/Users/marjank/code/apps-rendering/node_modules/ts-loader/dist/instances.js:288:9) WARN at successLoader (/Users/marjank/code/apps-rendering/node_modules/ts-loader/dist/index.js:26:17) WARN at Object.loader (/Users/marjank/code/apps-rendering/node_modules/ts-loader/dist/index.js:23:5) WARN @ \.)(?=.)[^/]*?\.stories\.(js|mdx|ts|tsx))$ (./src sync ^\.(?:(?:^|\/|(?:(?:(?!(?:^|\/)\.).)*?)\/)(?!\.)(?=.)[^/]*?\.stories\.(js|mdx|ts|tsx))$) ./components/anchor.stories.tsx WARN @ ./.storybook/generated-stories-entry.js WARN @ multi ./node_modules/@storybook/core/dist/server/common/polyfills.js ./node_modules/@storybook/core/dist/server/preview/globals.js ./.storybook/storybook-init-framework-entry.js ./node_modules/@storybook/addon-docs/dist/frameworks/common/config.js-generated-other-entry.js ./node_modules/@storybook/addon-docs/dist/frameworks/react/config.js-generated-other-entry.js ./node_modules/@storybook/addon-knobs/dist/preset/addDecorator.js-generated-other-entry.js ./node_modules/@storybook/addon-actions/dist/preset/addDecorator.js-generated-other-entry.js ./node_modules/@storybook/addon-actions/dist/preset/addArgs.js-generated-other-entry.js ./node_modules/@storybook/addon-backgrounds/dist/preset/addDecorator.js-generated-other-entry.js ./node_modules/@storybook/addon-backgrounds/dist/preset/addParameter.js-generated-other-entry.js ./.storybook/preview.js-generated-config-entry.js ./.storybook/generated-stories-entry.js

To Reproduce
Steps to reproduce the behavior:
1- Go to the PR for the webpack version change guardian/apps-rendering#1215
2- npm i
3- npm run storybook or npm run build-storybook

Expected behavior
It is supposed to build the ts-loader module successfully

System
"@storybook/addon-essentials": "^6.1.21",
"@storybook/addon-knobs": "^6.1.21",
"@storybook/addon-storyshots": "^6.1.21",
"@storybook/react": "^6.1.21",
"@storybook/storybook-deployer": "^2.8.7"
"webpack": "^5.24.2",
"webpack-cli": "^4.5.0",
"webpack-dev-server": "^3.11.2",
"html-webpack-plugin": "^5.2.0",
"ts-loader": "^8.0.17",

Node:: v12.20.0
NPM: 6.14.8

Additional context
Add any other context about the problem here.

@shilman
Copy link
Member

shilman commented Mar 9, 2021

webpack5 is not supported in 6.1.x. when i upgrade and follow the 6.2 webpack5 configuration instructions the ts-loader errors go away (and are replaced by some other errors, which might be related to your setup).

as a side note, i'm not sure what are the implications of using ts-loader in storybook, which already comes bundled with its own zero-config typescript setup based on babel. cc @mrmckeb

@JamieB-gu
Copy link

Thanks for getting back to us @shilman! We're aware Webpack 5 isn't officially supported yet, so appreciate you taking the time to look at this.

when i upgrade and follow the 6.2 webpack5 configuration instructions the ts-loader errors go away

Ah, I did naively attempt to bump our storybook dependencies to the latest 6.2.x beta, but didn't realise those instructions existed. We'll try those out and let you know what we get.

as a side note, i'm not sure what are the implications of using ts-loader in storybook

My understanding is that Storybook supports custom Webpack config? So far our experience has been that this works well with ts-loader - we've had this config in place for at least six months now I believe (using Webpack 4). We opted for ts-loader due to the caveats that come with the Babel plugin.

Regarding the Webpack 5 upgrade, I think we've narrowed down our issue to a check ts-loader does for the Webpack version. As far as I can tell, this check will report the version of Webpack you have installed at the top level of your project. In the case of a project that's updated to Webpack 5, this will send you down the Webpack 5 path in ts-loaders code. However, I think (correct me if I'm wrong) Storybook is actually using an instance of Webpack 4 at this point, so this code path crashes fairly quickly (because it attempts to look up an attribute that doesn't exist in Webpack 4). I can confirm that altering the check in ts-loader's code in order to force it into taking the Webpack 4 code-path fixes the problem.

It's possible that if ts-loader were able to check the version of the currently running instance of Webpack instead then this problem would go away. There does seem to be a way to do this in Webpack 5 because the Compiler exposes a webpack object which has that version field available, but I couldn't find a way to do this in Webpack 4.

@shilman
Copy link
Member

shilman commented Mar 9, 2021

@JamieB-gu in 6.1 it's all webpack4. in 6.2 it's webpack4 by default, but when you opt-in by installing @storybook/builder-webpack5 and setting core.builder to webpack5 in .storybook/main.js, it will use webpack5 instead of webpack4 for building your components/stories.

@JamieB-gu
Copy link

Thanks @shilman! Switching to that configuration seems to do the trick (as seen in the PR that GitHub has linked above ^).

I did have to tweak how terser-webpack-plugin was installed, which I believe is due to the issue discussed in #13893.

We'll do some testing to see if we can spot any issues caused by jumping to the beta.

@shilman
Copy link
Member

shilman commented Mar 9, 2021

Awesome @JamieB-gu !! Thanks for being a guinea pig on this 🙏

@JamieB-gu
Copy link

I'll try to keep the PR (linked above) updated with the betas you're releasing, to see if we can smoke out any issues.

Beta 14 seems to build fine according to our CI and works with our Chromatic integration (as seen in the PR checks). Only issue was the terser-plugin issue mentioned above: bumping the storybook versions essentially reverted the tweak I'd made to get it working - you can see what I did to fix that in the subsequent commit.

I don't know if there's a more sensible way to handle this when using npm. I'm not aware of any current Yarn "resolutions" equivalent, although looks like there's an RFC to add something similar: npm/rfcs#129

@webb04
Copy link

webb04 commented Mar 15, 2021

Thanks @JamieB-gu we had exactly the same issue 😀

@JamieB-gu
Copy link

We managed to get this merged in about a week ago (PR here in case it helps anyone in future: guardian/apps-rendering#1245) and it's been fine so far, so we're happy to close this issue now. Thanks for all your help @shilman!

Hi @webb04! Small world 🌎

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