Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for a JSON configuration file #814

Conversation

tmeasday
Copy link
Member

Note that I made a judgement on which keys to support in the file (see

projectId: z.string(),
projectToken: z.string(),
onlyChanged: z.union([z.string(), z.boolean()]),
onlyStoryFiles: z.array(z.string()),
onlyStoryNames: z.array(z.string()),
untraced: z.array(z.string()),
externals: z.array(z.string()),
ci: z.boolean(),
debug: z.boolean(),
junitReport: z.union([z.string(), z.boolean()]),
autoAcceptChanges: z.union([z.string(), z.boolean()]),
exitZeroOnChanged: z.union([z.string(), z.boolean()]),
exitOnceUploaded: z.union([z.string(), z.boolean()]),
ignoreLastBuildOneBranch: z.string(),
buildScriptName: z.string(),
outputDir: z.string(),
storybookBuildDir: z.string(),
storybookBaseDir: z.string(),
storybookConfigDir: z.string(),
ownerName: z.string(),
repositorySlug: z.string(),
})
) - which may be wrong, I am very open to ideas here.

@linear
Copy link

linear bot commented Sep 11, 2023

AP-3623 Add CLI support for a chromatic.js config file that allows multiple environments and configuration outside of .storybook/

Currently users must configure their chromatic cli inside of storybook configuration and through cli flags. This may be confusing as users might require different settings for local builds vs ci builds. We may be able to improve this by allowing users to define their configurations in a separate, type-safe config file.

This should be an additional form of configuration, and allow backwards compatibility for existing setups using cli flags and ci configuration. It may need to allow both approaches to be mixed.

Multiple Environments:

With the addon-visual-tests having more users run builds outside of CI, configuration for the cli may need to vary based on the environment. Supporting multiple environments, through environment-suffixed names or programatic configuration would help optimize configuration for local builds.

Related Slack thread: https://chromaticqa.slack.com/archives/C051TQR6QLC/p1693198584200749

Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the filename, but otherwise looks good.

package.json Outdated Show resolved Hide resolved
node-src/ui/messages/errors/missingConfigurationFile.ts Outdated Show resolved Hide resolved
node-src/ui/messages/errors/invalidConfigurationFile.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmeasday left some questions for you that we can follow async to address the documentation accordingly. I'm approving this to unblock you on this.

node-src/lib/getConfiguration.ts Outdated Show resolved Hide resolved
node-src/lib/getConfiguration.ts Show resolved Hide resolved
Copy link
Contributor

@weeksling weeksling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a question and a couple suggestions about using the flag to override configuration instead of the other way around.

node-src/index.ts Show resolved Hide resolved
node-src/lib/getOptions.ts Outdated Show resolved Hide resolved
@tmeasday tmeasday force-pushed the tom/ap-3623-add-cli-support-for-a-chromaticjs-config-file-that-allows branch from 6f0837c to 5460c6c Compare September 13, 2023 01:55
@tmeasday tmeasday merged commit a0b14d8 into main Sep 14, 2023
18 of 20 checks passed
@tmeasday tmeasday deleted the tom/ap-3623-add-cli-support-for-a-chromaticjs-config-file-that-allows branch September 14, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants