feat: integrate Storybook into the project#7204
Open
fairlighteth wants to merge 4 commits intodevelopfrom
Open
feat: integrate Storybook into the project#7204fairlighteth wants to merge 4 commits intodevelopfrom
fairlighteth wants to merge 4 commits intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds Storybook (config, stories, scripts, deps), integrates Storybook build into CI with notifications, removes two button variants (ButtonLight, ButtonSecondaryAlt), updates ButtonSecondary and related fixtures/usages, and ignores Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner
participant PNPM as pnpm / Storybook build
participant Notify as notify-failure job
participant Slack as Slack webhook
GH->>Runner: start workflow (setup -> needs)
Runner->>PNPM: run storybook job (install, pnpm run storybook:build)
PNPM-->>Runner: storybook result (success/failure)
Runner->>Notify: job results including needs.storybook.result
Notify->>Slack: send aggregated failure message (includes STORYBOOK_RESULT)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.storybook/main.ts (2)
92-94: Consider relocatinggetAbsolutePathbefore its usage.The function is defined after it's used on lines 73-75. While JavaScript hoisting makes this work, placing helper functions before their call sites improves readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/main.ts around lines 92 - 94, Move the helper function getAbsolutePath so it is declared before its first use to improve readability; locate the getAbsolutePath function and the code that calls it (the import.meta.resolve + dirname usage) and cut the existing function definition and paste it above the code block that invokes getAbsolutePath, keeping the same signature function getAbsolutePath(value: string): string { return dirname(fileURLToPath(import.meta.resolve(`${value}/package.json`))) } and preserving imports (dirname, fileURLToPath, import.meta.resolve) so behavior remains unchanged.
30-30: Consider defensive parsing for package.json files.
JSON.parsewill throw on malformed JSON. While unlikely in a controlled monorepo, adding a try-catch would make the build more resilient to corrupted or WIP package files.🛡️ Proposed defensive parsing
function getWorkspaceAlias(packageJsonPath: string): [string, string] | null { if (!existsSync(packageJsonPath)) { return null } - const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as WorkspacePackageJson + let packageJson: WorkspacePackageJson + try { + packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as WorkspacePackageJson + } catch { + return null + } const entryPoint = getWorkspaceEntryPoint(packageJson)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/main.ts at line 30, Wrap the JSON.parse call that produces packageJson from packageJsonPath in a try-catch to defensively handle malformed package.json; inside the catch, log or surface the parse error (include packageJsonPath and error) and set packageJson to a safe fallback typed as WorkspacePackageJson (e.g., an empty object cast) or rethrow a clearer error—update the code around the packageJson = JSON.parse(...) expression so packageJson and its use-sites (symbol: packageJson) remain typed while avoiding an uncaught exception from JSON.parse.libs/ui/src/pure/Button/Button.stories.tsx (2)
79-90: Minor duplication withButtonUsage.stories.tsx.
InlineBannerUsagePreviewreplicates the banner pattern fromButtonUsage.stories.tsx. Consider importingInlineBannerSecondaryActionstory or extracting a shared component if maintaining sync becomes a burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui/src/pure/Button/Button.stories.tsx` around lines 79 - 90, InlineBannerUsagePreview duplicates the banner pattern found in ButtonUsage.stories.tsx; replace the duplication by reusing the existing story or shared component: import the InlineBannerSecondaryAction story/component from ButtonUsage.stories (or extract a new shared InlineBanner component) and use that inside Button.stories.tsx instead of recreating the JSX, ensuring you reference InlineBannerUsagePreview (to remove) and InlineBannerSecondaryAction or the new InlineBanner component (to import/use) so both stories stay in sync.
1-246: File is at 246 LOC — close to the 250-line limit.This comprehensive story file with a custom docs page justifies the length, but consider splitting if more stories are added. The custom
ButtonDocsPage(lines 92-112) could move to a separateButton.docs.tsxfile if the story count grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui/src/pure/Button/Button.stories.tsx` around lines 1 - 246, The file is nearing the 250-line limit; extract the custom docs page into its own module to keep the story file smaller: create a new file that exports the existing ButtonDocsPage function (keep the JSX and any helper components it uses like InlineBannerUsagePreview and the docs strings), update the story meta to import and reference the exported ButtonDocsPage in parameters.docs.page, and remove the ButtonDocsPage implementation from Button.stories.tsx (leaving any small helpers used only by docs either moved with it or replaced by imports). Ensure symbols referenced are ButtonDocsPage, InlineBannerUsagePreview (if moved), and the meta object’s parameters.docs.page so the story continues to use the external docs component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 20-21: The Storybook npm scripts "storybook" and "storybook:build"
currently set env vars using POSIX syntax which fails on Windows; update both
scripts to prefix their environment assignments with cross-env (e.g., use
cross-env STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home ...) so that
the variables are applied cross-platform when running the "storybook" and
"storybook:build" scripts.
---
Nitpick comments:
In @.storybook/main.ts:
- Around line 92-94: Move the helper function getAbsolutePath so it is declared
before its first use to improve readability; locate the getAbsolutePath function
and the code that calls it (the import.meta.resolve + dirname usage) and cut the
existing function definition and paste it above the code block that invokes
getAbsolutePath, keeping the same signature function getAbsolutePath(value:
string): string { return
dirname(fileURLToPath(import.meta.resolve(`${value}/package.json`))) } and
preserving imports (dirname, fileURLToPath, import.meta.resolve) so behavior
remains unchanged.
- Line 30: Wrap the JSON.parse call that produces packageJson from
packageJsonPath in a try-catch to defensively handle malformed package.json;
inside the catch, log or surface the parse error (include packageJsonPath and
error) and set packageJson to a safe fallback typed as WorkspacePackageJson
(e.g., an empty object cast) or rethrow a clearer error—update the code around
the packageJson = JSON.parse(...) expression so packageJson and its use-sites
(symbol: packageJson) remain typed while avoiding an uncaught exception from
JSON.parse.
In `@libs/ui/src/pure/Button/Button.stories.tsx`:
- Around line 79-90: InlineBannerUsagePreview duplicates the banner pattern
found in ButtonUsage.stories.tsx; replace the duplication by reusing the
existing story or shared component: import the InlineBannerSecondaryAction
story/component from ButtonUsage.stories (or extract a new shared InlineBanner
component) and use that inside Button.stories.tsx instead of recreating the JSX,
ensuring you reference InlineBannerUsagePreview (to remove) and
InlineBannerSecondaryAction or the new InlineBanner component (to import/use) so
both stories stay in sync.
- Around line 1-246: The file is nearing the 250-line limit; extract the custom
docs page into its own module to keep the story file smaller: create a new file
that exports the existing ButtonDocsPage function (keep the JSX and any helper
components it uses like InlineBannerUsagePreview and the docs strings), update
the story meta to import and reference the exported ButtonDocsPage in
parameters.docs.page, and remove the ButtonDocsPage implementation from
Button.stories.tsx (leaving any small helpers used only by docs either moved
with it or replaced by imports). Ensure symbols referenced are ButtonDocsPage,
InlineBannerUsagePreview (if moved), and the meta object’s parameters.docs.page
so the story continues to use the external docs component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 25cc1acb-fe63-46a5-b294-51a488acb624
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/workflows/ci.yml.gitignore.storybook/main.ts.storybook/preview.tsx.storybook/tsconfig.jsonapps/cowswap-frontend/src/common/pure/CustomRecipientWarningBanner/index.tsxlibs/ui/src/index.tslibs/ui/src/pure/Button/Button.stories.tsxlibs/ui/src/pure/Button/ButtonMod.tsxlibs/ui/src/pure/Button/ButtonUsage.stories.tsxlibs/ui/src/pure/Button/__fixtures__/Button.tsxlibs/ui/src/pure/Button/index.tsxlibs/ui/src/pure/ButtonSecondaryAlt/index.tsxlibs/ui/tsconfig.lib.jsonpackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (3)
- libs/ui/src/pure/Button/ButtonMod.tsx
- libs/ui/src/pure/ButtonSecondaryAlt/index.tsx
- libs/ui/src/index.ts
Comment on lines
+20
to
+21
| "storybook": "STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook dev -p 6006", | ||
| "storybook:build": "STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook build --output-dir build/storybook", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify scripts that use inline env assignment without cross-env
python - <<'PY'
import json, re
with open("package.json") as f:
pkg = json.load(f)
for name, cmd in pkg.get("scripts", {}).items():
if re.match(r'^[A-Z_]+=.*', cmd) and "cross-env" not in cmd:
print(f"{name}: {cmd}")
PYRepository: cowprotocol/cowswap
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Check if cross-env is already a dependency in package.json
jq '.devDependencies, .dependencies' package.json | grep -i "cross-env"Repository: cowprotocol/cowswap
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Check for all scripts using POSIX-style env assignment without cross-env
python - <<'PY'
import json, re
with open("package.json") as f:
pkg = json.load(f)
all_issues = []
for name, cmd in pkg.get("scripts", {}).items():
if re.match(r'^[A-Z_]+=', cmd) and "cross-env" not in cmd:
all_issues.append(f"{name}: {cmd}")
if all_issues:
print("Scripts with POSIX env assignment (no cross-env):")
for issue in all_issues:
print(f" {issue}")
else:
print("No other issues found")
PYRepository: cowprotocol/cowswap
Length of output: 325
Use cross-env for Storybook scripts to keep cross-platform execution reliable.
Lines 20–21 currently use POSIX-style env assignment; these commands fail in Windows shells. Since cross-env is already a dependency, wrap both scripts:
- "storybook": "STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook dev -p 6006",
- "storybook:build": "STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook build --output-dir build/storybook",
+ "storybook": "cross-env STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook dev -p 6006",
+ "storybook:build": "cross-env STORYBOOK_DISABLE_TELEMETRY=1 HOME=.cache/storybook-home storybook build --output-dir build/storybook",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 20 - 21, The Storybook npm scripts "storybook" and
"storybook:build" currently set env vars using POSIX syntax which fails on
Windows; update both scripts to prefix their environment assignments with
cross-env (e.g., use cross-env STORYBOOK_DISABLE_TELEMETRY=1
HOME=.cache/storybook-home ...) so that the variables are applied cross-platform
when running the "storybook" and "storybook:build" scripts.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a monorepo-root Storybook pilot for libs/ui, documents the shared Button surface, and removes dead/duplicate button variants as part of the initial component audit workflow.
This PR:
To Test
Background
This is the first small pilot for using Storybook as the component audit/documentation surface.
Notes:
Follow-up PRs are expected to:
Summary by CodeRabbit
New Features
Refactor
Chores