CLI: Improve package incompatibility detection and warning#34559
Conversation
…at canary eslint-plugin-storybook as compatible - Replace vague "TypeScript < 4.9 or incompatible tooling" warning with detailed per-check failure messages listing which packages are below required versions - Treat Storybook canary/prerelease eslint-plugin-storybook versions (0.0.0-*) as compatible since they are development builds, not outdated stable releases - Add tests for single-tool failure messages and canary version handling Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/03429f2f-58e1-4038-abe3-5bf054c6e37b Co-authored-by: yannbf <1671563+yannbf@users.noreply.github.com>
|
@copilot the message is still a bit misleading because it says "populating javascript examples" but the function is just detecting the language type. The javascript explanation should me moved up to where it makes the most sense. |
…e function only detects language Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/80bff2fc-8f6f-41a4-b594-af7818bc61da Co-authored-by: yannbf <1671563+yannbf@users.noreply.github.com>
Updated the warning to |
|
@copilot don't you think that function is supposed to detect language and this message about fallback to javascript is in the wrong place? |
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/427e0f3f-5743-49c5-9033-b8dab6a237ef Co-authored-by: yannbf <1671563+yannbf@users.noreply.github.com>
…aration of concerns Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/427e0f3f-5743-49c5-9033-b8dab6a237ef Co-authored-by: yannbf <1671563+yannbf@users.noreply.github.com>
Good call. |
|
@copilot the language detection should be clean, and there should be a separate function for incompatibility detection |
…ions() into two clean functions Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/234aa538-77b9-452c-834c-9419ca6a7730 Co-authored-by: yannbf <1671563+yannbf@users.noreply.github.com>
Separated into two clean functions: |
📝 WalkthroughWalkthroughProjectTypeService refactors language detection by extracting package compatibility checking into a reusable ChangesLanguage Detection & Package Compatibility Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/lib/create-storybook/src/services/ProjectTypeService.ts (1)
265-267: 💤 Low valueInconsistent version checking pattern for Prettier.
The Prettier version check uses
semver.gte(line 265) while all other package checks use thesatisfieshelper with range strings. This inconsistency might be intentional, but it's worth verifying.
satisfies(version, '>=2.8.0')would be more consistent with the pattern used for TypeScript, Babel, and ESLint packages.♻️ Proposed fix for consistency
- if (prettierVersion && !semver.gte(prettierVersion, '2.8.0')) { + if (prettierVersion && !satisfies(prettierVersion, '>=2.8.0')) { incompatibleReasons.push(`prettier ${prettierVersion} is below 2.8.0`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/lib/create-storybook/src/services/ProjectTypeService.ts` around lines 265 - 267, Replace the inconsistent Prettier check that uses semver.gte with the project's standard satisfies helper: instead of calling semver.gte(prettierVersion, '2.8.0'), use satisfies(prettierVersion, '>=2.8.0') when deciding whether to push the incompatibleReasons entry (look for the prettierVersion variable, the incompatibleReasons array, and the current semver.gte call inside ProjectTypeService.ts) so the Prettier check matches the TypeScript/Babel/ESLint pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/lib/create-storybook/src/commands/ProjectDetectionCommand.ts`:
- Around line 56-70: The current detectAndReportLanguage implementation logs a
single combined warning, but tests expect two separate logger.warn calls; update
detectAndReportLanguage so when language === SupportedLanguage.JAVASCRIPT and
incompatibleReasons.length > 0 it first calls logger.warn with the generic
message "Populating with JavaScript examples due to incompatible package
versions:" and then calls logger.warn a second time with the formatted list of
reasons (e.g., incompatibleReasons.map(r => ` - ${r}`).join('\n')), leaving all
other logic (projectTypeService.detectLanguage and
projectTypeService.detectIncompatiblePackageVersions) unchanged.
---
Nitpick comments:
In `@code/lib/create-storybook/src/services/ProjectTypeService.ts`:
- Around line 265-267: Replace the inconsistent Prettier check that uses
semver.gte with the project's standard satisfies helper: instead of calling
semver.gte(prettierVersion, '2.8.0'), use satisfies(prettierVersion, '>=2.8.0')
when deciding whether to push the incompatibleReasons entry (look for the
prettierVersion variable, the incompatibleReasons array, and the current
semver.gte call inside ProjectTypeService.ts) so the Prettier check matches the
TypeScript/Babel/ESLint pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 910a0dbd-2550-4852-8d73-13dcc7da3a83
📒 Files selected for processing (4)
code/lib/create-storybook/src/commands/ProjectDetectionCommand.test.tscode/lib/create-storybook/src/commands/ProjectDetectionCommand.tscode/lib/create-storybook/src/services/ProjectTypeService.test.tscode/lib/create-storybook/src/services/ProjectTypeService.ts
storybook initemits "Detected TypeScript < 4.9 or incompatible tooling" when any bundled version check fails—even if TypeScript is fine and only e.g. Prettier (from a monorepo root) is below threshold. Additionally, canaryeslint-plugin-storybookversions (0.0.0-pr-*) fail>=0.6.8since the numeric part is0.0.0.Manual testing
Changes
detectLanguage()is now a pure detection function returningSupportedLanguage. A newdetectIncompatiblePackageVersions()method separately checks installed tooling versions against TypeScript compatibility constraints and returnsstring[]with specific failure reasons. The "Populating with JavaScript examples" warning is inProjectDetectionCommand, where the language result is actually used:eslint-plugin-storybookversion gate for0.0.0-*versions (Storybook PR/canary builds where the numeric version is meaningless)detectLanguagetests assert plain language return values;detectIncompatiblePackageVersionstests cover compatible, incompatible, single-tool failure, and canary bypass scenarios;ProjectDetectionCommandtests verify the warning is logged by the callerSummary by CodeRabbit
Release Notes