-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
CLI: Capture the version specifier used in create-storybook
#32344
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
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.
3 files reviewed, 2 comments
ancestry: ReturnType<typeof getProcessAncestry> | ||
): string | undefined { | ||
for (const ancestor of ancestry) { | ||
const match = ancestor.command?.match(/[ \-]storybook@([^\s]+)/); |
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.
style: The regex pattern could potentially match false positives. Consider using a more specific pattern like /(?:^|\s|-)storybook@([^\s]+)/
to ensure proper word boundaries.
const ancestry = [{ command: 'node' }, { command: '[email protected]' }, { command: 'npm' }]; | ||
expect(getStorybookVersionFromAncestry(ancestry as any)).toBeUndefined(); | ||
}); |
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.
style: Test name 'possible storybook path' is unclear - consider renaming to 'returns undefined for storybook command without create/init keywords'
const ancestry = [{ command: 'node' }, { command: '[email protected]' }, { command: 'npm' }]; | |
expect(getStorybookVersionFromAncestry(ancestry as any)).toBeUndefined(); | |
}); | |
it('returns undefined for storybook command without create/init keywords', () => { | |
const ancestry = [{ command: 'node' }, { command: '[email protected]' }, { command: 'npm' }]; | |
expect(getStorybookVersionFromAncestry(ancestry as any)).toBeUndefined(); | |
}); |
View your CI Pipeline Execution ↗ for commit f3e5667
☁️ Nx Cloud last updated this comment at |
398da27
to
d97fd4d
Compare
Make more specific to match Also search in reverse order because |
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.
LGTM, only one minor nit.
Co-authored-by: Jeppe Reinhold <[email protected]>
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
Before | After | Difference | |
---|---|---|---|
Dependency count | 48 | 48 | 0 |
Self size | 30.74 MB | 30.62 MB | 🎉 -119 KB 🎉 |
Dependency size | 17.61 MB | 17.61 MB | 0 B |
Bundle Size Analyzer | Link | Link |
@storybook/cli
Before | After | Difference | |
---|---|---|---|
Dependency count | 204 | 219 | 🚨 +15 🚨 |
Self size | 879 KB | 879 KB | 🎉 -90 B 🎉 |
Dependency size | 81.72 MB | 81.82 MB | 🚨 +104 KB 🚨 |
Bundle Size Analyzer | Link | Link |
@storybook/codemod
Before | After | Difference | |
---|---|---|---|
Dependency count | 173 | 188 | 🚨 +15 🚨 |
Self size | 35 KB | 35 KB | 🎉 -12 B 🎉 |
Dependency size | 76.79 MB | 76.89 MB | 🚨 +100 KB 🚨 |
Bundle Size Analyzer | Link | Link |
create-storybook
Before | After | Difference | |
---|---|---|---|
Dependency count | 49 | 49 | 0 |
Self size | 1.52 MB | 1.52 MB | 🚨 +5 KB 🚨 |
Dependency size | 48.35 MB | 48.23 MB | 🎉 -119 KB 🎉 |
Bundle Size Analyzer | node | node |
create-storybook
…fier CLI: Capture the version specifier used in `create-storybook` (cherry picked from commit 98c4d9a)
Closes N/A.
What I did
Introduces logic to detect and report the Storybook version used to initiate a project by inspecting the ancestry of process commands. The main changes add a dependency for process ancestry inspection, implement a utility to extract the Storybook version from the ancestry, and update telemetry reporting to include this version specifier. Comprehensive tests were added to verify the new utility
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
STORYBOOK_TELEMETRY_DEBUG=1
🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>