-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: handle invalid directory errors when creating atomic component #1327
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned Manifest Files |
Thanks for your contribution @dmbrooke !
|
Pull Request Report PR Title ✅ Title follows the conventional commit spec. |
if (cwdFiles.length > 0 && !hasPackageInCwd) { | ||
handleErrors( | ||
new InvalidProjectDirectory( | ||
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.' |
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.
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.' | |
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in a valid project' |
maybe specifying how to initialize an atomic project
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.
Maybe link to some relevant doc which explains the process e.g https://docs.coveo.com/en/cli/ or something more specific if available
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.
Disagreeing here: The create-atomic-(result)-component packages are initializers that are agnostic of the concept of atomic project.
i.e. you can use them without an atomic project.
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.
I personally think it's valid to refer to it as a valid project
as that isn't specific to an atomic project. I think I'll update the link for this description to instead point to the atomic:cmp
command documentation as it clearly outlines in what contexts the command will work:
https://docs.coveo.com/en/cli/#coveo-atomiccmp-name
if (cwdFiles.length > 0 && !hasPackageInCwd) { | ||
handleErrors( | ||
new InvalidProjectDirectory( | ||
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.' |
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.
Maybe link to some relevant doc which explains the process e.g https://docs.coveo.com/en/cli/ or something more specific if available
KIT-2722
Unfortunately adding to the pile that is CDX-1428, but I couldn't find any simple/quick way to share the error class and behaviour between packages without doing larger changes.
Proposed changes
It would be useful to provide a helpful error message when attempting to create an atomic component in an invalid directory. Thus, I added the necessary to the
create-atomic-component
andcreate-atomic-result-component
packages.Testing
UNIT TESTS ARE STILL IN PROGRESS. For now, you can test the changes by following these steps:
npm run build
)