Codemod: Fix glob pattern handling on Windows#33714
Conversation
|
View your CI Pipeline Execution ↗ for commit 5f4ed14
☁️ Nx Cloud last updated this comment at |
7757879 to
6bb0e14
Compare
Normalize backslashes to forward slashes in glob patterns before passing them to tinyglobby. On Windows, path.join() produces backslash-separated paths, but glob libraries require forward slashes. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
6bb0e14 to
7ef33d0
Compare
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughImports Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
✨ Finishing touches
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Comment |
Codemod: Fix glob pattern handling on Windows (cherry picked from commit 4d07f57)
Closes #
What I did
Fixed a Windows-specific bug where
runCodemodin@storybook/codemodfailed to find files becausetinyglobbyrequires forward slashes in glob patterns, butpath.join()on Windows produces backslash-separated paths.The Bug
When running codemods on Windows, files were silently not being transformed because:
C:\Users\...\file.stories.tsx(or usespath.join()to construct it)tinyglobbyrequires forward slashes for glob patternsRoot Cause
The
cli-storybookversion ofrunCodemodalready handles this correctly using theslashpackage:But the
@storybook/codemodversion was missing this normalization:The Fix
Use
pathe.normalize()(already used throughout the codebase) to normalize paths before passing totinyglobby:How this was discovered
This pre-existing bug was exposed by the unit tests added in #33646. The tests use
path.join(tmpdir(), ...)which produces backslash paths on Windows, causing the tests to fail on Windows CI (ci:daily). The bug itself has existed longer - it would have affected any Windows user calling the codemod with a backslash path.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
This was caught by the existing unit tests in
src/index.test.tswhich were failing on Windows CI (ci:daily). The fix makes those tests pass. This affects real Windows users, not just tests - anyone usingnpx storybook migrateon Windows with backslash paths would be affected.Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 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/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.