[WEB-4725] chore: storybook setup & tailwind config package improvements#7614
[WEB-4725] chore: storybook setup & tailwind config package improvements#7614sriramveeraghanta merged 9 commits intopreviewfrom
Conversation
|
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 You can disable this status message by setting the WalkthroughAdds Storybook to the propel package (configs, scripts, Turbo task, gitignore), introduces a classname utility and utils barrel, and adds a global Tailwind CSS theming file and related UI utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant NPM as NPM Scripts
participant SB as Storybook (React+Vite)
participant Turbo as Turbo
participant FS as Filesystem
Dev->>NPM: npm run storybook
NPM->>SB: start Storybook dev server (port 6006)
SB-->>Dev: serves stories from ../src/**/*.stories.@(ts|tsx)
Note right of SB: loads global CSS from @plane/tailwind-config/global.css
Dev->>NPM: npm run build-storybook
NPM->>SB: build static site
SB->>FS: write output to storybook-static/**
Turbo->>SB: build-storybook dependsOn ^build (orchestrates)
FS-->>Dev: storybook-static available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tailwind-config/tailwind.config.js (1)
1-3: convertToRGB returns rgba(...) without alpha — invalid CSS; use rgb(var(...)) instead.Today,
rgba()requires an alpha component;rgba(23,23,23)is invalid. We should outputrgb(var(--token))for 3‑component variables and usergba(var(--token), <alpha>)only when we need opacity.Apply this change at the top helpers:
-const convertToRGB = (variableName) => `rgba(var(${variableName}))`; +const convertToRGB = (variableName) => `rgb(var(${variableName}))`; const convertToRGBA = (variableName, alpha) => `rgba(var(${variableName}), ${alpha})`;Note: This aligns with how tokens are defined in global.css (triplets for rgb and occasional quartets where alpha is intended). After this change, scan for any call sites that relied on the previous (invalid) rgba behavior.
🧹 Nitpick comments (10)
packages/tailwind-config/tailwind.config.js (1)
21-21: Likely wrong content glob due to content.relative=true; remove or correct "./propel/".**With
content.relative: true, globs are resolved relative to this config file’s directory (packages/tailwind-config)."./propel/**/*"would point topackages/tailwind-config/propel/**, which likely doesn’t exist. You already include the correct workspace path at Line 23 ("../../packages/propel/src/**/*.{js,ts,jsx,tsx}"). Suggest removing the redundant/incorrect glob and (optionally) excluding Propel stories to avoid shipping story-only classes.Apply this diff:
@@ - "./propel/**/*.{js,ts,jsx,tsx}", "../../packages/ui/src/**/*.{js,ts,jsx,tsx}", "../../packages/propel/src/**/*.{js,ts,jsx,tsx}", "../../packages/editor/src/**/*.{js,ts,jsx,tsx}", - "!../../packages/ui/**/*.stories{js,ts,jsx,tsx}", + "!../../packages/ui/**/*.stories{js,ts,jsx,tsx}", + "!../../packages/propel/**/*.stories{js,ts,jsx,tsx}",packages/propel/package.json (2)
12-14: LGTM: Storybook scripts are correct for v9 CLI.
storybook dev -p 6006andstorybook buildare the right commands. Consider adding a "preview-storybook" script if you routinely serve the static build, but not required.
34-41: Dependencies for cn utility look good; double-check intended public surface.Adding
clsxandtailwind-mergemakes sense for thecnutility. Ifcnis meant to be consumed externally, consider adding an explicit export path (e.g.,"./utils": "./src/utils/index.ts") so consumers don’t import internal paths. If it’s internal-only, ignore.packages/tailwind-config/global.css (2)
382-421: Move demo/feature-specific styles (tags input) out of global design tokens.These are component/demo-specific rules and bloat global CSS for all consumers. Consider relocating to Storybook-only CSS or the component package’s scoped stylesheet.
I can open a follow-up PR moving these into a Storybook preview CSS file if helpful.
475-592: Spinner block is sizable and opinionated; consider localizing it.Same rationale as tags input. Unless used across apps, prefer colocating this in the specific feature (or Storybook) to keep global CSS lean.
packages/propel/src/utils/index.ts (1)
1-1: Prefer explicit named/type re-exports overexport *for stabilityBarrel looks fine, but explicit exports reduce accidental surface area and ease tree-shaking. Also handy to re-export
ClassValueso consumers can type their helpers.Apply:
-export * from "./classname"; +export { cn } from "./classname"; +export type { ClassValue } from "clsx";packages/propel/src/utils/classname.tsx (1)
1-4: Add explicit return type and consider.tsextension (no JSX)Implementation is standard (clsx + tailwind-merge) and correct. Two small polish items:
- Type the return explicitly to improve API clarity in IDEs.
- Since there’s no JSX, rename to
.tsto avoid unnecessary.tsxtooling paths.Suggested tweak:
-import { clsx, type ClassValue } from "clsx"; +import { clsx, type ClassValue } from "clsx"; import { twMerge } from "tailwind-merge"; -export const cn = (...inputs: ClassValue[]) => twMerge(clsx(inputs)); +export const cn = (...inputs: ClassValue[]): string => twMerge(clsx(inputs));If you want, I can add a tiny test matrix for
cn(strings, arrays, conditionals, duplicates merging).packages/propel/.storybook/preview.ts (1)
1-3: ImportPreviewtype from@storybook/reactfor framework-agnostic typingDepending on the Storybook version, types are exposed from
@storybook/react(commonly recommended), while the framework runner stays@storybook/react-vite. This avoids future churn if you switch renderers.-import type { Preview } from "@storybook/react-vite"; +import type { Preview } from "@storybook/react"; import "@plane/tailwind-config/global.css";Please confirm the project’s SB major version; I can adjust this if you’re pinned to a variant that exports the type from
react-vite.packages/propel/.storybook/main.ts (2)
9-11: Avoidanyand makerequire.resolveESM-safe
getAbsolutePath’s return type can bestring.- In ESM Storybook environments,
requiremight be undefined. UsecreateRequireto keep Yarn PnP/monorepo resolution robust.-import { join, dirname } from "path"; +import { join, dirname } from "node:path"; +import { createRequire } from "node:module"; @@ -function getAbsolutePath(value: string): any { - return dirname(require.resolve(join(value, "package.json"))); -} +function getAbsolutePath(value: string): string { + const r = typeof require === "undefined" ? createRequire(import.meta.url) : require; + return dirname(r.resolve(join(value, "package.json"))); +}If your TS/build setup guarantees CJS here, the ESM guard is harmless; if not, it prevents runtime failures.
13-19: Add@storybook/addon-essentialsso configured Controls actually show up
preview.tsconfiguresparameters.controls.matchers, but no addons are enabled. Without Essentials, Controls (and other commonly expected features) won’t render.const config: StorybookConfig = { stories: ["../src/**/*.stories.@(ts|tsx)"], - addons: [], + addons: [getAbsolutePath("@storybook/addon-essentials")], framework: { name: getAbsolutePath("@storybook/react-vite"), options: {}, }, };If you prefer a slimmer set, we can include only
@storybook/addon-controlsand friends explicitly—say the word.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.gitignore(1 hunks)packages/propel/.eslintrc.js(1 hunks)packages/propel/.storybook/main.ts(1 hunks)packages/propel/.storybook/preview.ts(1 hunks)packages/propel/package.json(2 hunks)packages/propel/src/utils/classname.tsx(1 hunks)packages/propel/src/utils/index.ts(1 hunks)packages/tailwind-config/global.css(1 hunks)packages/tailwind-config/tailwind.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-04T16:22:44.344Z
Learnt from: lifeiscontent
PR: makeplane/plane#7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
Applied to files:
packages/propel/.storybook/main.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
.gitignore (1)
100-101: LGTM: ignore storybook-static (matches anywhere in the repo).The plain pattern "storybook-static" will ignore any directory with that name at any depth (e.g., packages/propel/storybook-static), which is exactly what we want for Storybook builds.
packages/propel/.eslintrc.js (1)
4-4: LGTM: Enable Storybook ESLint rules.Extending
plugin:storybook/recommendedis consistent with the new Storybook setup and the devDependency has been added. No further changes needed.packages/propel/package.json (1)
46-51: Version alignment: Storybook 9 packages are consistent; ensure repo-wide tooling matches.
storybook,@storybook/react-vite, andeslint-plugin-storybookare all on 9.1.x—good. Verify that any shared scripts/tooling (root package, CI) aren’t pinning Storybook 8.x, and that Vite plugins in the workspace are compatible.packages/propel/.storybook/main.ts (1)
1-19: Good: Vite framework, minimal config, Yarn PnP-friendly resolverThis mirrors Storybook’s recommended v8 monorepo pattern and avoids the webpack CSS loader overrides that we keep in
packages/uiper prior learnings. No action needed here.
lifeiscontent
left a comment
lifeiscontent
left a comment
There was a problem hiding this comment.
a good start, but needs work ✨
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
turbo.json (2)
41-44: Exclude storybook-static from lint inputs to avoid scanning generated artifactsLinting over compiled Storybook output can slow CI and produce noisy results. You already exclude .next and dist—extend that to storybook-static.
"check:lint": { "cache": false, - "inputs": ["**/*", "!**/.next/**", "!**/dist/**"] + "inputs": ["**/*", "!**/.next/**", "!**/dist/**", "!**/storybook-static/**"] },
28-31: Optional: define inputs for build-storybook for tighter cache keysTurbo will infer inputs, but being explicit can make cache behavior clearer and exclude bulky, irrelevant dirs.
- "build-storybook": { - "dependsOn": ["^build"], - "outputs": ["storybook-static/**"] - }, + "build-storybook": { + "dependsOn": ["build", "^build"], + "inputs": ["**/*", "!**/.next/**", "!**/dist/**", "!**/storybook-static/**", "!**/coverage/**"], + "outputs": ["storybook-static/**"] + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/propel/.storybook/main.ts(1 hunks)packages/propel/.storybook/preview.ts(1 hunks)packages/propel/package.json(3 hunks)packages/tailwind-config/tailwind.config.js(1 hunks)turbo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/propel/.storybook/main.ts
- packages/propel/.storybook/preview.ts
- packages/tailwind-config/tailwind.config.js
- packages/propel/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
turbo.json (2)
28-31: LGTM: Task name and outputs align with Storybook defaultsThe pipeline task name matches the package script and the outputs pattern correctly captures the Storybook static build directory. This will enable deterministic Turbo caching and pairs well with the repo’s .gitignore update.
28-31: No local build dependency needed for Propel’s StorybookAfter inspecting packages/propel/.storybook and the package’s entry points, we can confirm:
- The Storybook config (
main.ts) directly loads stories from the local src tree ("../src/**/*.stories.@(ts|tsx)") and resolves components via Vite at runtime.- There are no imports from dist or from
@plane/propelin any stories or preview files.- The package.json exports point to source files, not to built artifacts.
Because Storybook is bundling the source files in src directly and does not consume the compiled outputs, adding
"build"to thedependsOnarray will not affect the Storybook build. The existing^builddependency already ensures upstream packages are built before Storybook runs.You can safely ignore the suggested change to
"dependsOn": ["build", "^build"].
lifeiscontent
left a comment
lifeiscontent
left a comment
There was a problem hiding this comment.
we should definitely try to clean more of this up soon.
3 participants
Description
This PR includes the following changes:
Summary by CodeRabbit
New Features
Chores
Refactor