search

chromaui / addon-visual-tests

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

Ignore `storybookBuildDir` property for VTA builds #324

Closed ghengeveld closed 6 days ago

ghengeveld commented 1 week ago

VTA doesn't support prebuilt Storybooks, because there's no way to trigger a rebuild when starting a VTA build through Storybook. Using storybookBuildDir would cause VTA to run against the exact same prebuilt Storybook each time, which isn't expected. Therefore, this updates the config overrides to unset storybookBuildDir.

ndelangen commented 6 days ago

I think this makes sense. I assume VTA will complain when there's no storybookBuildScript it can run at all?

yannbf commented 6 days ago

I think this change would only make sense if the user understand what is happening. If this silently fails it's not so great!

ndelangen commented 6 days ago

The status quo is that VTA keeps publishing the same storybook, right? That's even worse.

What makes you think this will silently fail? I'd assume it would fail hard is it can't build storybook.

(if the command is unknown, it will try a default, AFAIK)

ghengeveld commented 6 days ago

VTA defers to the Chromatic CLI, which has various config checks in place, so that's covered. It doesn't silently fail if it can't build the Storybook, it'll show a notification and refer to the Storybook CLI output which will show more details (provided by Chromatic CLI).

It's actually really uncommon to have storybookBuildDir as a config value, usually it would be passed as a flag (--storybook-build-dir) in which case VTA wouldn't be involved. This PR is just to address the edge case of people configuring storybookBuildDir through their chromatic config file, which makes sense for CI but doesn't make sense for VTA.