-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error messaging and user guidance when there's a path mismatch between components pull and types generate commands. The changes focus on providing clearer feedback when component JSON files are not found due to directory path misalignment.
- Enhanced JSDoc documentation for
ReadComponentsOptionsto clarify path requirements for type generation - Added a
purposefield to distinguish between push-components and types-generate flows - Improved error messages with context-specific guidance and directory path information
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/cli/src/commands/components/push/constants.ts | Enhanced JSDoc documentation and added purpose field to ReadComponentsOptions interface |
| packages/cli/src/commands/components/push/actions.ts | Updated readComponentsFiles function with improved error handling and context-specific messaging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const { from, path, separateFiles = false, suffix } = options; | ||
| options: ReadComponentsOptions, | ||
| ): Promise<ComponentsData> => { | ||
| const { from, path = '.storyblok', separateFiles = false, suffix, purpose = 'push-components' } = options; |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
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.
| const { from, path = '.storyblok', separateFiles = false, suffix, purpose = 'push-components' } = options; | |
| const { from, path = '.storyblok/components', separateFiles = false, suffix, purpose = 'push-components' } = options; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.

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
There was a problem hiding this comment.
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
Description
This PR improves the user experience in the
types generateflow when the component JSON files are not found due to a path mismatch. Previously, users might hit a vague "file not found" error if:--path, buttypes generatewas run without the same--path.types generatewas run with a custom--path.What’s Changed
ReadComponentsOptionsJSDoc:pathis optional, but fortypes generateit must always match the path used when runningcomponents pull.readComponentsFilesto explicitly referencestoryblok types generatevsstoryblok components pull.Looked for components in: ...).components.jsonis found in the resolved path while runningtypes generate.Notes
push componentsflow.