Vite: Fix config loading - project directory

[nVitius]

I haven't filed an issue for this yet. Happy to create one if there is any value in tracking it that way.

What I did

When calling Vite's loadConfigFromFile there is an option to specify the configRoot. That is, the directory that contains the package's package.json. By default, this is process.cwd().
If you run storybook from a directory that is not the package root, then this will be the wrong directory. This is a common strategy when running storybook in a monorepo, for example.

Vite uses the package.json to determine if it is an ESM and changes its bundling strategy accordingly. I was running into an issue where a package imported into my vite.config.ts was failing to import correctly when running Storybook, but not when building with Vite directly.

This PR adds the correct root directory to loadConfigFromFile.

How to test

1. Run a storybook project using the vite builder

Checklist

• Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
• Make sure to add/update documentation regarding your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Maintainers

• If this PR should be tested against many or all sandboxes,
  make sure to add the ci:merged or ci:daily GH label to it.
• Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

[nVitius]

@JReinhold can you re-approve the workflows on this PR, please?

[IanVS]

Thanks for the PR, this looks good to me! I think our heuristic for determining where the root actually is might be a bit brittle, but it's the way we've been doing it for a while and we haven't had reports of problems. And we can try to find a better way in the future if we do hear from folks.

Thanks again!

[nVitius]

@IanVS Just wanted to follow up on this PR. I went ahead and rebased over the latest next branch.

[nVitius]

@IanVS Just wanted to follow up on this.
There was a merge conflict with next that I've resolved just now. Please let me know if there's anything else I can do to help move this along.

[IanVS]

@nVitius what do you think about Norbert's suggestion to use getProjectRoot? That seems like a better approach, to me.

[ndelangen]

@IanVS I made the change, but it can no longer find:

ERR! Error: [vite]: Rollup failed to resolve import "@storybook/react/preview" from "/virtual:/@storybook/builder-vite/vite-app.js".
ERR! This is most likely unintended because it can break your application at runtime.

[IanVS]

@ndelangen I think this is failing because in our monorepo, the .git folder is not actually in the sandbox project root. So, I think this could also fail in a monorepo situation. So maybe the vcs root isn't a good heuristic for the project root after all. I think we can revert your last commit, and land this how it was. It's an incremental solution that doesn't solve the issue of a nested storybook config directory, but I think it does solve the original issue that the OP was trying to solve (vite not using the correct project root to resolve the config).

[swernerx]

Interestingly this broke our mono repo build. We have the vite.config.ts in the individual app/package folders but store the actual storybook config in a package storybook-config. This dramatically simplifies using Storybook all over the other packages.

In here the framework/options section fixed discovering our Vite config - this was not required before:

import path from "node:path"

import type { StorybookConfig } from "@storybook/react-vite"

const config: StorybookConfig = {
  stories: [path.join(process.cwd(), "src/**/*.stories.@(ts|tsx)")],
  addons: [
    "@storybook/addon-links",
    "@storybook/addon-essentials",
    "@storybook/addon-interactions"
  ],
  framework: {
    name: "@storybook/react-vite",
    options: {
      builder: {
        viteConfigPath: path.join(process.cwd(), "vite.config.ts")
      }
    }
  },

  typescript: {
    check: false
  }
}

export default config

We also use a wrapper for the storybook binary to simplify using this config:

const path = require("path")
const { symlinkSync } = require("fs")
const { spawn } = require("child_process")

const configFolder = path.resolve(__dirname, "../config")
const binPath = path.resolve(__dirname, "../node_modules/.bin/storybook")

const ports = {
  uikit: 8000,
  theme: 8100,
  workbench: 9000,
  dashboard: 9100
}

;(async () => {
  const command = process.argv[2]
  const params = [
    command,
    "--config-dir",
    configFolder,
    "--disable-telemetry",
    ...process.argv.slice(3)
  ]

  if (command === "dev") {
    const base = path.basename(process.cwd())
    params.push("--port", ports[base] ?? 7000)
    params.push("--no-open")
  }

  if (command === "build") {
    params.push("--output-dir", "public")
  }

  try {
    const cmd = spawn(binPath, params)

    // Reduce noise by not piping over stdout
    cmd.stdout.pipe(process.stdout)

    cmd.stderr.pipe(process.stdout)
    await cmd
  } catch (err) {
    console.log("Storybook Error:", err)
    process.exitCode = 1
  }
})()

This is mainly meant as a side-node for other people looking for a solution if this also broke for them.

[gustaff-weldon]

I think this PR breaks resolution of vite.config.ts

[gustaff-weldon]

@ndelangen @IanVS

Reposting as main comment for a better visibility:

I think this change actually broke resolution of vite.config file.
In 7.1 our vite.config.ts file which lives in the root of our project was resolved just fine. I could see our custom plugins loaded fine when inspecting config passed to viteFinal.

From 7.2 the passed config contains only storybook plugins and our project vite.config is not resolved

I have tried passing the viteConfigPath (as instructed in https://storybook.js.org/docs/builders/vite) relative to resolved projectRoot, (also relative to project root) but to no avail:

  core: {
    options: {
      viteConfigPath: '../vite.config.ts' // didn't work
      // viteConfigPath: 'vite.config.ts' // didn't work either
    }
  },

Also, running with or without builder: '@storybook/builder-vite', in core does not make any difference in behaviour. In 7.1 we were running just fine without that option.

It is not clear why in the first place there's an assumption that path.resolve(options.configDir, '..'); resolves to project root.
In our case eg. storybook config lives in config/storybook/ folder, (storybook dev -c config/storybook) so project root resolves to config/ folder which certainly is not a project root.

Somehow that assumption worked in 7.1, but with this PR broke vite config resolution.

I think @ndelangen had a point when asked for a better project root resolution. Cc @IanVS

Update:
Looks like either docs are wrong or this has changed since 7.2, the options with config path need to be under builder (found out after switching our main.js to TS):

  core: {
    builder: {
      name: '@storybook/builder-vite',
      options: {
        viteConfigPath: 'vite.config.ts'
      }
    }
  },

With setting the viteConfigPath I was able to override config path but the the broken automatic vite config resolution introduced in this PR still exists.