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.

Sign up for GitHub

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

CLI: Add support for custom vite config to autoblocker #26087

Merged
merged 2 commits into from
Feb 20, 2024
+15 −6
Merged

CLI: Add support for custom vite config to autoblocker #26087

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1307cd3
resolve https://github.com/storybookjs/storybook/pull/25934#pullreque…
ndelangen Feb 19, 2024
4ee0478
change var to boolean
ndelangen Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions code/lib/cli/src/automigrate/fixes/vite-config-file.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ export const viteConfigFile = {
id: 'viteConfigFile',

async check({ mainConfig, packageManager }) {
const viteConfigPath = await findUp([
let isViteConfigFileFound = !!(await findUp([
'vite.config.js',
'vite.config.mjs',
'vite.config.cjs',
'vite.config.ts',
]);
]));

const rendererToVitePluginMap: Record<string, string> = {
preact: '@preact/preset-vite',
Expand All @@ -45,7 +45,16 @@ export const viteConfigFile = {

const rendererName = frameworkToRenderer[frameworkName as keyof typeof frameworkToRenderer];

if (!viteConfigPath && isUsingViteBuilder) {
if (
!isViteConfigFileFound &&
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a scenario here where the user specifies a custom path to a file that doesn't exist, which will return existed: true but there won't be a file.

This is probably an edge case, but I have seen support comments suggesting this as a way to disable auto-loading of the root Vite config entirely, so it's not completely unrealistic to get in this situation.

Copy link
Member Author

@ndelangen ndelangen Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I feel like it's not the blocker's responsibility to verify if the user configured this correctly; do you?

mainConfig.core?.builder &&
typeof mainConfig.core?.builder !== 'string' &&
mainConfig.core?.builder.options
) {
isViteConfigFileFound = !!mainConfig.core?.builder.options.viteConfigPath;
Comment on lines +50 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

couldn't this be simplified a little by just doing optional-chaining all the way down?

Suggested change
mainConfig.core?.builder &&
typeof mainConfig.core?.builder !== 'string' &&
mainConfig.core?.builder.options
) {
isViteConfigFileFound = !!mainConfig.core?.builder.options.viteConfigPath;
) {
isViteConfigFileFound = !!mainConfig.core?.builder?.options?.viteConfigPath;

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be. Although I personally like the explicit style here a bit more because it makes clear, that builder can be a string or object.

}

if (!isViteConfigFileFound && isUsingViteBuilder) {
const plugins = [];

if (rendererToVitePluginMap[rendererName]) {
Expand All @@ -54,7 +63,7 @@ export const viteConfigFile = {

return {
plugins,
existed: !!viteConfigPath,
existed: isViteConfigFileFound,
};
}

Expand All @@ -66,7 +75,7 @@ export const viteConfigFile = {

const pluginVersion = await packageManager.getPackageVersion(plugin);

if (viteConfigPath && isUsingViteBuilder && !pluginVersion) {
if (isViteConfigFileFound && isUsingViteBuilder && !pluginVersion) {
const plugins = [];

if (plugin) {
Expand All @@ -75,7 +84,7 @@ export const viteConfigFile = {

return {
plugins,
existed: !viteConfigPath,
existed: !isViteConfigFileFound,
};
}

Expand Down
Loading