'yarn start-storybook' creates node_modules/.cache in Yarn 2 PnP project

[ulrik-s]

Describe the bug
My project is using Yarn 2 PnP and consequently there exists no node_modules folder.

Running yarn start-storybook inside my storybook project creates a node_modules/.cache/storybook/* folder structure with files related to babel inside it.

The problematic code is in lib/core/src/server/utils/resolve-path-in-sb-cache.js:

export function resolvePathInStorybookCache(fileOrDirectoryName) {
  const cwd = process.cwd();
  const projectDir = pkgDir.sync(cwd);

  let cacheDirectory;

  if (!projectDir) {
    cacheDirectory = path.resolve(cwd, '.cache/storybook');
  } else {
    cacheDirectory = path.resolve(projectDir, 'node_modules/.cache/storybook');
  }

  return path.join(cacheDirectory, fileOrDirectoryName);
}

Since I have a package.json in this sub-project folder projectDir will contain that path and the else-branch will be taken.

The correct route to solve this? Making cacheDirectory a configurable variable? Check
if node_modules already exists before entering the else-clause? Maybe a combination of both?

I have tried both strategies and they both seem to work. Arguably it is more user friendly if things worked right out of the box which would favour the second solution.

[shilman]

@gaetanmaisse WDYT?

[ulrik-s]

I forgot to mention that I am on 6.0.0-beta.21

[gaetanmaisse]

I updated the path of the cache directory when I worked on Yarn 2 compatibility as it was previously: node_modules/@storybook/core/.cache -> 💥 with PnP (you can take a look at the three 1st commits of #9667).

When I did this change I looked for a "classic/conventional" way to handle cache but unfortunately, I didn't find any great things. So I discussed a bit with some other maintainers and we decided to go with node_modules/.cache/@storybook because:

• This is the "pattern" (node_modules/.cache/the-package) used by some known and widely used libs (I can't find the list we did when we discussed that)
• node_modules folder is already in the .gitignore of all JS/TS projects
• Devs are used to have "throwable" things in node_modules and sometimes try to remove it to "fix" dependencies issues (especially when updating deps)
• It just works with NPM, Yarn 1/2 + PnP or not

I also asked a Yarn maintainer if he was aware of other projects with the same issue/question. Good news we are not alone 😃 but there isn't any convention for now 😞. However, @arcanis (🧶 Yarn main maintainer) has found in this "issue" a potential opportunity to create an official Yarn package for cache management, details are here: yarnpkg/berry#918. Having such a package would be great both for Storybook and the whole JS ecosystem, so lets 🤞😃

Did you face any issue with the current behavior? or were you just surprised to have node_modules folder back?

[arcanis]

Yep, node_modules/.cache is kind of the de-facto convention for package caches at the moment (that's also what babel-loader does, for example).

Note that Yarn 2 intentionally doesn't remove dot-folders from the node_modules even in PnP mode, so it shouldn't have adverse impact on your app - just keep the n_m folder gitignored.

[ulrik-s]

You are right, everything works but I was surprised to find node_modules in my project directory.
I just made the assumption it was a case that was overlooked. Thank you for the clarification.

[shilman]

@gaetanmaisse @arcanis just want to say you guys rock! it's absolutely amazing that you're on top of this. 💯💯💯