chromaui / addon-visual-tests

Visual Tests addon for Storybook
MIT License
28 stars 1 forks source link

Chromatic CLI and VTR in monorepos have conflicting interpretation of the chromatic.config.json's `storybookBaseDir` #292

Closed vicrep closed 1 month ago

vicrep commented 2 months ago

Bug report

Hi there! This concerns both Chromatic and the VTA, so posting here as a catch-all.

I'm working on a mono-repo, where one of our packages has chromatic + SB set up, which on its own runs well on CI

Structure:

packages/
  my-package/
    .storybook/...
    package.json

CI (example):

- run: cd packages/my-package && npx chromatic --exit-once-uploaded --only-changed

Now, we're trying to add the Visual Testing Addon to said mono-repo package, which as part of onboarding, make us create a chromatic.config.json. This file is added in package/my-package/chromatic.config.json. Following onboarding and what the VTA tells us we need to locally run our visual tests, we're made to set the configuration as such:

{
  "projectId": "Project:...",
  "storybookBaseDir": "packages/my-package",
}

This is where the conflict arises. Running chromatic (CLI) now complains that it can't find the baseDir of our stories, as it seems to be trying to find it from the current working directory. Changing the storybookBaseDir to "." (or removing it completely) fixes the chromatic run (and is what is suggested by running npx @chromatic-com/turbosnap-helper), but then, VTA starts to show the Visual tests locked due to configuration problem. error, and suggest that we set the baseDir to "packages/my-package".

Note that in those examples, all commands (storybook, chromatic, etc) are being run from the project root (packages/my-package), not the repo root.

We've worked around this by renaming the config file to chromatic.config.local.json and customizing the VTA to use that file instead, but this conflicting behaviour sounds like something that should be addressed as it's bound to lead to more confusion and makes it hard to have a universal chromatic configuration for use by both the CLI and the VTA.

This might relate somehow to this feature ticket as well: https://github.com/chromaui/addon-visual-tests/issues/255

Please let me know if I can provide you with any more information on this / our setup. Thanks in advance!

sroy3 commented 2 months ago

The conflict is also popping up locally when adding onlyChanged to the config file for the Visual Testing Addon.

vicrep commented 2 months ago

Looking at the source code for the VTA, it seems that this check is the culprit: https://github.com/chromaui/addon-visual-tests/blob/7d6d6c6575c4f7a07be3f38c0ddbe5fc6386b4e7/src/preset.ts#L87

Which is not something that is checked by the CLI at this stage, IIUC.

JReinhold commented 2 months ago

Yeah this sounds like an unintended behavior in VTA, or a monorepo use-case that hasn't been considered in the current implementation.

ghengeveld commented 1 month ago

The issue is actually in the Chromatic CLI. The VTA is suggesting the correct value here. I've opened a PR to fix it.