Skip to content

chore(cli): improve UX when generating types with mismatched component paths #334

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(cli): improve UX when generating types with mismatched component paths #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 31 additions & 10 deletions packages/cli/src/commands/components/push/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,24 +247,45 @@ export const upsertComponentInternalTag = async (
return await pushComponentInternalTag(space, tag);
}
};

export const readComponentsFiles = async (
options: ReadComponentsOptions): Promise<ComponentsData> => {
const { from, path, separateFiles = false, suffix } = options;
options: ReadComponentsOptions,
): Promise<ComponentsData> => {
const { from, path = '.storyblok', separateFiles = false, suffix, purpose = 'push-components' } = options;
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The default value for path should be '.storyblok/components' to match the documented default, not '.storyblok'. This mismatch could cause the function to look in the wrong directory.

Suggested change
const { from, path = '.storyblok', separateFiles = false, suffix, purpose = 'push-components' } = options;
const { from, path = '.storyblok/components', separateFiles = false, suffix, purpose = 'push-components' } = options;

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@alvarosabu alvarosabu Oct 6, 2025

Choose a reason for hiding this comment

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

blocker:

Hi @rydkvist, this is relevant, I believe, the default path should be .storyblok/components

Copy link
Contributor Author

@rydkvist rydkvist Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hey! No, not really, it should not be .storyblok/components

The resolvePath function in line 254, sets .storyblok as the default path if no custom path is passed.
image

This means if path is undefined, then .storyblok will be set today already.

I personally didn't feel this was clear when working with this piece of code (specifically in the readComponentsFiles function). To make this more explicitly, I went ahead and added the fallback one level above, on readComponentsFiles.

The resolvePath function is generic, it's used in multiple action files, and personally I would make the path parameter a required one, so each action consuming the resolvePath has an intentional path, instead of a default path .storyblok which can be missused and lead to future confusion

So basically my goal with this change was to explicitly state that the readComponentsFiles function controls the default path, and not theresolvePath function itself.
So that in the future, when we want to change the default file path for this, we do it on this level and not in the resolvePath level

Copy link
Contributor Author

@rydkvist rydkvist Oct 7, 2025

Choose a reason for hiding this comment

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

On the other hand, all of the other actions dont have a explicit fallback like I added here. So the proper way to handle this could be to revisit all use cases where resolvePath is used, and decide if we actually want to have a fallback in place, or if we should remove the fallback inside resolvePath and instead have the functions calling resolvePath passing a required path

const resolvedPath = resolvePath(path, `components/${from}`);

// Check if directory exists first
try {
await readdir(resolvedPath);
}
catch (error) {
const message = `No local components found for space ${chalk.bold(from)}. To push components, you need to pull them first:

1. Pull the components from your source space:
${chalk.cyan(`storyblok components pull --space ${from}`)}

2. Then try pushing again:
${chalk.cyan(`storyblok components push --space <target_space> --from ${from}`)}`;
const err = error as NodeJS.ErrnoException;
const notFound = err?.code === 'ENOENT';
const header = notFound
? `No local components directory found for space ${chalk.bold(from)}.`
: `Failed to read local components for space ${chalk.bold(from)}.`;

const nextStep = purpose === 'types-generate'
? `To generate types, the CLI needs the local component JSON files.
If you ran ${chalk.cyan('storyblok components pull')} with a custom ${chalk.bold('--path')},
you must use the same path again when running ${chalk.cyan('storyblok types generate')}.

Example:
${chalk.cyan(`storyblok components pull --space ${from} --path ./custom-folder`)}
${chalk.cyan(`storyblok types generate --space ${from} --path ./custom-folder`)}`
: `To push components you need the local component JSON files.
Pull them first with:
${chalk.cyan(`storyblok components pull --space ${from}`)}

Then run your push command again:
${chalk.cyan(`storyblok components push --space <target_space> --from ${from}`)}`;

const message = `${header}

The CLI attempted to read component files from:
${chalk.cyan(resolvedPath)}

${nextStep}
`;

throw new FileSystemError(
'file_not_found',
Expand Down
14 changes: 13 additions & 1 deletion packages/cli/src/commands/components/push/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ export interface PushComponentsOptions extends CommandOptions {

export interface ReadComponentsOptions extends PushComponentsOptions {
/**
* The path to read the components file from.
* Local path where component JSON files are located.
*
* ⚠️ Important: When using types generation command,
* if you provide a custom `--path`, it must be the same
* `--path` that was used when running
* `storyblok components pull`.
*
* Defaults to `.storyblok/components`.
* @default `.storyblok/components`
*/
Expand All @@ -35,4 +41,10 @@ export interface ReadComponentsOptions extends PushComponentsOptions {
* Target space
*/
space?: string;
/**
* Determines the flow:
* - `push-components`: pushing local components to Storyblok
* - `types-generate`: generating type definitions from local JSON files
*/
purpose?: 'push-components' | 'types-generate';
}
Loading