Storybook: 3.3.6: ERROR in ./node_modules/@storybook/addon-knobs/src/react/index.js Module parse failed: Unexpected token (25:9)

[joscha]

Issue details

When updating from 3.3.5 to 3.3.6, this error appears:

ERROR in ./node_modules/@storybook/addon-knobs/src/react/index.js
Module parse failed: Unexpected token (25:9)
You may need an appropriate loader to handle this file type.
|   const initialContent = getStory(context);
|   const props = { context, storyFn: getStory, channel, knobStore, initialContent };
|   return <WrapStory {...props} />;
| };
|
 @ ./node_modules/@storybook/addon-knobs/dist/register.js 3:13-29
 @ ./node_modules/@storybook/addon-knobs/register.js
 @ ./conf/storybook/addons.js
 @ multi ./node_modules/@storybook/react/dist/server/config/polyfills.js ./conf/storybook/addons.js ./node_modules/@storybook/react/dist/client/manager/index.js

It seems to be related to the src folder being exposed, via https://github.com/storybooks/storybook/pull/2678.

When rm -rf node_modules/@storybook/addon-knobs/src/ this fixes the problem.
Strangely none of the other add-ons seem affected:

cat conf/storybook/addons.js
import '@storybook/addon-actions/register';
import '@storybook/addon-knobs/register';
import '@storybook/addon-options/register';
import '@storybook/addon-viewport/register';

Steps to reproduce

just start storybook

Please specify which version of Storybook and optionally any affected addons that you're running

• @storybook/react 3.3.6

├─ @storybook/[email protected]
├─ @storybook/[email protected]
├─ @storybook/[email protected]
├─ @storybook/[email protected]
├─ @storybook/[email protected]

via yarn list | grep @storybook

[joscha]

We import '@storybook/addon-knobs/register'; in addons.js. I am not following, how we get from the chain addons.js -> addon-knobs/register.js -> addon-knobs/dist/register.js suddenly to addon-knobs/src/react/index.js

[ndelangen]

Might be a babel configuration issue. Seems like Object spread might be disabled on your babel config for some reason?

[joscha]

Seems like Object spread might be disabled on your babel config for some reason?

We don't have babel at all.

[ndelangen]

Actually 25:9 points to the start of a JSX tag

[joscha]

Seems like Object spread might be disabled on your babel config for some reason?

Are you suggesting storybook is now BYO babel? Did that happen from 3.2 to 3.3 and it only surfaces now in 3.3.6 when src/ is available? That would make sense, however I think exposing src/ would then mean that needs to be at least a patch, maybe even a major version. It still does not explain, however how we get from the register.js through dist/register.js suddenly to src/react/index.js?

[joscha]

Actually 25:9 points to the start of a JSX tag

yes, I don't think it really matters what symbol it is - there could be all kinds of things in the ES6 source - the question is why it suddenly pulls in es6 source, where it should use the dist.

[joscha]

I wonder if it is related to https://github.com/storybooks/storybook/issues/2680#issuecomment-356245262 - @Hypnosphi WDYT?

[ndelangen]

@joscha That is not really the goal.

Though adding babel to your setup would solve the immediate problem you have, there's the question whether this is a good idea.

We expose our src to enable users to bundle more efficiently and run with the native features, like async/await etc. BUT our src contains JSX, and this is not valid JS.

I think we need to address this on our side. AKA not publish our source, but instead an JS:next version of it (transpiling the JSX).

[joscha]

Though adding babel to your setup would solve the immediate problem you have, there's the question whether this is a good idea.

Definitely not, we are fully Typescript, babel is needed nowhere :)

We expose our src to enable users to bundle more efficiently and run with the native features, like async/await etc. BUT our src contains JSX, and this is not valid JS.

That makes sense, but do you have any idea why it would suddenly use that? My understanding is that the main field points to the dist, which should be neatly transpiled, whereas the jsnext:main points to the src/ (and should not be used at all unless I tell webpack to pick up jsnext:main).

[ndelangen]

@joscha Why it choose to use the src over dist, is because of this in the package.json:

"jsnext:main": "src/index.js",

I'm pretty sure, webpack will decide which file to use when bundling.
This is configurable

[joscha]

@joscha Why it choose to use the src over dist, is because of this in the package.json:

"jsnext:main": "src/index.js",

That was my first hunch as well, but I believe that not to be correct because the webpack target is "web" which sets the mainFields (https://webpack.js.org/configuration/resolve/#resolve-mainfields) to "browser", "module", "main". This does not seem to be overridden anywhere in storybook and also not by us.

[Hypnosphi]

@joscha how does your webpack config for storybook look like? Is there a chance you lose the first loader from base config?

Oh nevermind, it isn’t configured to transpile node_modules anyway. I think we should just revert all the jsnext:main fields for now

[Hypnosphi]

storybook is now BYO babel

Unfortunately, it kinda is, but only in the sense that you need babel-core as dev dependency (we moved it to peers for compatibility with different babel versions)

[joscha]

Oh nevermind, it isn’t configured to transpile node_modules anyway. I think we should just revert all the jsnext:main fields for now

I think that would be good, then at least we wouldn't break existing behaviour.

Unfortunately, it kinda is, but only in the sense that you need babel-core as dev dependency (we moved it to peers for compatibility with different babel versions)

I think that is unfortunate, it makes the assumption that people using storybook will use babel. That is definitely not always the case. Also, this babel would need to be configured, right? How would a consumer with no babel in the rest of the codebase configure such a babel? By inheriting loaders from the default webpack config? In any case, I think this needs some documentation for upgraders from 3.2 to 3.3.x.

[Hypnosphi]

this babel would need to be configured

Not really, you only need the package to be present

this needs some documentation for upgraders from 3.2 to 3.3.x.

Sure: https://github.com/storybooks/storybook/pull/2709

[Hypnosphi]

The weirdest part about it is that dist/register doesn't actually require anything with "react" in its name except for react package itself (line 3).

Is there a chance that you use some webpack resolve magic so that (require('react') resolves to ../src/react?

[TroySchmidt]

I have the same exact issue with 3.4.0-alpha.1 I also am using 100% TypeScript based React app. The project was created with create react app typescript and then ejected. And if babel-core is a peer dependency then npm install certainly isn't giving me that peer dependency warning but it is giving me one that @storybook/addons@^3.3.0 is not installed.

@joscha Have you seen any good Storybook alternatives? Unfortunately this isn't the first configuration issue I have had trying to use Storybooks with TypeScript and I am losing confidence that they will be resolved. It seems that as of late things are centered around compliance with Angular.

[TroySchmidt]

Using exact version of 3.3.3 doesn't have this issue. addon-knobs does have a peer dependency of @angular/core, but I am using React / TypeScript so that probably should be optional peer dependency instead.

[tmeasday]

As bizarre as it is, I am inclined to agree with @Hypnosphi that it seems like the problem is require('react') is resolving to require('../src/react').

@joscha / @TroySchmidt any chance of a reproduction or some information on what your webpack config for storybook looks like? Specifically your require.alias field or anything like that?

[tmeasday]

Unfortunately this isn't the first configuration issue I have had trying to use Storybooks with TypeScript and I am losing confidence that they will be resolved.

@TroySchmidt obviously as more and more different types of apps are supported things will slip through the cracks (still don't even quite know which crack has been slipped through in this case!). Always keen to have more eyes and helpers to test and resolve issues with different combinations of configuration in the weird and wonderful world of JS!

[strothj]

This seems to work as a temporary workaround, haven't tested extensively.

config.plugins.push(
    new webpack.NormalModuleReplacementPlugin(/^react$/, resource => {
      if (!/addon-knobs/.test(resource.context)) return;
      resource.request = require.resolve("react");
    }),
  );

[TroySchmidt]

I wish I was smarter in the innards of Webpack and what is happening. The only special Webpack I have to work with typescript is it imports the Webpack from storybook and appends a loader for TypeScript ts-loader.
The other issue is that using another machine to build via VSTS the output is not registering and loading the add-ons. The addon file is responding with a console.log but for whatever reason it doesn't work. I deleted npm cache, fresh node_modules folder, delete package-lock.json. I don't know of another method to debug what is happening differently in that CI environment than happens on my local machine.

[strothj]

So I managed to create a repository to reproduce this issue:
https://github.com/strothj/storybook-issue-2704

I believe the issue is caused when the Webpack resolve module setting is overridden:
https://webpack.js.org/configuration/resolve/#resolve-modules

If you set modules to ["node_modules", "src"] then you're telling Webpack to try resolve the module name react in the current directory, then work its way up. The knobs directory has a directory src with a matching package react.

I think the proper way to override this setting in a custom Webpack config would be something like this:

config.resolve.modules = ["node_modules", path.resolve(__dirname, "../src")];

[Hypnosphi]

Yes, you definitely should use an absolute path for this option

[TroySchmidt]

That fixed it!! I remember what happened now. The path include wasn't there on the Webpack configuration example when I added it so I removed the path.resolve from the push onto config.resolve.modules.

[Hypnosphi]

@TroySchmidt sorry, I can't find a resolve.modules field there

[TroySchmidt]

@Hypnosphi hahaha I just looked again and updated the comment while you were typing that response. I looked at the history and the const path = require("path") was added after I copied that code over. So when I ran it since it complained about path I just removed the path.resolve(__dirname, '../') from my webpack.config.js file and it just worked. But since that time I guess something changed with knobs in a refactoring that exposed that problem. Apparently I wasn't the only person to resolve their configuration file that way.
I am going to test this with our CI build via VSTS and see if that fixes the output not showing or recognizing the addons channel.

[joscha]

we have a "src" in our config.resolve.modules as well. Will try and verify that that is the problem. If yes, will update here and close the issue.

[tmeasday]

I wonder where this is coming from. Is there a documentation issue somewhere? Let's try to fix it.

[joscha]

making src absolute seems to fix it. I suppose it comes from https://webpack.js.org/configuration/resolve/#resolve-modules where it says:

Absolute and relative paths can both be used, but be aware that they will behave a bit differently.

so I think it would just be a problem for upgraders where it previously worked by accident (e.g. we never intended to have src picked up in any subdir).