Addon-docs: Resolve babel-loader from storybook/core

[shilman]

Issue: #13593

What I did

Resolve babel-loader from @storybook/core so we don't need to install it and potentially break CRA. This is a workaround for what appears to be a yarn bug.

How to test

CI build

self-merging @tooppaaa @mrmckeb

[merceyz]

This is a workaround for what appears to be a yarn bug.

If the parent of @storybook/addon-docs doesn't declare babel-loader as a dependency then this is expected

[shilman]

@merceyz what's the parent of @storybook/addon-docs?

when i do yarn list it shows babel-loader at the root, but when i look in the node_modules, babel-loader is actually located inside subprojects (unhoisted), so definitely something fishy going on.

[gaetanmaisse]

what's the parent of @storybook/addon-docs?

@shilman it's either the user's project or @storybook/addon-essential.

I think everything was working fine because sb init was adding babel dependencies before #13220 has been merged.

[shilman]

Thanks for clarifying. If that's the case I'm surprised we're only hearing about this error now, unless I missed it (very possible!). Either way, this seems to fix the problem. Is there a more proper way to do it?

[mrmckeb]

Amazing work @shilman!

[merceyz]

@merceyz what's the parent of @storybook/addon-docs?

Running yarn create react-app foo && cd foo && yarn dlx sb init shows that it's the users project, which storybook stopped installing babel-loader into in #13220, so it not working makes perfect sense, PnP would have stopped this one but that isn't covered in the e2e test atm

yarn why babel-loader -R
└─ foo@workspace:.
   ├─ @storybook/addon-docs@npm:6.1.11 [6847a] (via npm:^6.1.11 [6847a])
   │  └─ @storybook/core@npm:6.1.11 [25a5a] (via npm:6.1.11 [25a5a])
   │     └─ babel-loader@npm:8.2.2 [bb165] (via npm:^8.0.6 [bb165])
   ├─ @storybook/addon-essentials@npm:6.1.11 [6847a] (via npm:^6.1.11 [6847a])
   │  └─ @storybook/addon-docs@npm:6.1.11 [22d5d] (via npm:6.1.11 [22d5d])
   │     └─ @storybook/core@npm:6.1.11 [25a5a] (via npm:6.1.11 [25a5a])
   ├─ @storybook/react@npm:6.1.11 [6847a] (via npm:^6.1.11 [6847a])
   │  └─ @storybook/core@npm:6.1.11 [25a5a] (via npm:6.1.11 [25a5a])
   └─ react-scripts@npm:4.0.1 [6847a] (via npm:4.0.1 [6847a])
      └─ babel-loader@npm:8.1.0 [7f8b9] (via npm:8.1.0 [7f8b9])

Is there a more proper way to do it?

It's perfectly fine to resolve dependencies from one of your dependencies as long as you know they'll continue to depend on it to not suddenly break in a patch release. That said you shouldn't have to, create-react-app shouldn't care what is in node_modules as the package manager places everything they need where it needs to be either way.

when i do yarn list it shows babel-loader at the root, but when i look in the node_modules, babel-loader is actually located inside subprojects (unhoisted), so definitely something fishy going on.

It's not hoisted to the root because of dependency conflicts and you can't rely on it to be hoisted either (hello PnP), if you look at the output above you see the actual dependency graph and based on just that @storybook/addon-docs doesn't have access

[shilman]

thanks so much for the clarification @merceyz i'm a little bit slow on all this, but i appreciate your patience. we'll get there! 🙏