React: Add isPackage flag to component imports for better package identification #33090
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GC as getComponents
participant RSR as cachedResolveImport
participant RDG as getReactDocgen
participant RW as getImports / rewriter
GC->>RSR: resolve import specifier
alt non-relative import resolves
RSR-->>GC: resolved path
note right of GC `#DFF2E1`: set component.isPackage = true
else relative or unresolved
RSR-->>GC: relative path or error
note right of GC `#FFEFD5`: component.isPackage = false
end
GC->>RDG: call getReactDocgen(component with isPackage)
RDG-->>GC: reactDocgen metadata
GC->>RW: compute imports(component)
alt component.isPackage == true
RW-->>GC: do not rewrite import to packageName
else
RW-->>GC: allow rewrite relative -> package import
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
195-215: Consider consolidating duplicate resolution logic.The code resolves the same import twice (lines 197-204 and 207-214) with similar logic. Consolidate into a single resolution block to improve efficiency and maintainability.
.map((component) => { let path; let isPackage = false; try { if (component.importId && storyFilePath) { path = cachedResolveImport(matchPath(component.importId, dirname(storyFilePath)), { basedir: dirname(storyFilePath), }); + // Detect package imports: non-relative imports resolving to node_modules + const isNonRelative = !component.importId.startsWith('.') && !component.importId.startsWith('/'); + if (isNonRelative && path.includes('node_modules')) { + isPackage = true; + } } } catch (e) { logger.debug(e); } - try { - if (component.importId && !component.importId.startsWith('.') && storyFilePath) { - const realPath = cachedResolveImport(component.importId, { - basedir: dirname(storyFilePath), - }); - if (realPath.includes('node_modules')) { - isPackage = true; - } - } - } catch {} - const componentWithPackage = { ...component, isPackage };
276-276: Unused helper function.The
isRelativehelper is defined but never used in this file. Consider removing it or integrating it into the package detection logic if it was intended to improve the non-relative import checks.If unused, apply this diff:
- const isRelative = (id: string) => id.startsWith('.') || id === '.'; - const withSource = components
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/renderers/react/src/componentManifest/getComponentImports.ts(5 hunks)code/renderers/react/src/componentManifest/valid-package-name.ts(0 hunks)
💤 Files with no reviewable changes (1)
- code/renderers/react/src/componentManifest/valid-package-name.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
code/renderers/react/src/componentManifest/utils.ts (1)
cachedResolveImport(93-93)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)getImportTag(297-301)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
19-19: LGTM: Clean API extension.The
isPackagefield is a clear addition to the public API that supports the new package detection feature.
298-305: LGTM: Correct rewriting logic.The conditional logic correctly prevents rewriting imports that are already packages when
packageNameis provided. The comment clearly explains the intent.
|
View your CI Pipeline Execution ↗ for commit e84b50c
☁️ Nx Cloud last updated this comment at |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
206-215: Package detection remains fragile despite previous feedback.The
includes('node_modules')check is still susceptible to issues with:
- pnpm's
.pnpm/directory structure- Yarn PnP (no node_modules)
- Symlinked dependencies
- Monorepo workspace: protocol
The empty catch block continues to hide resolution errors.
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
279-279: Unused helper function.The
isRelativehelper is defined but doesn't appear to be called anywhere in the visible code. Consider removing it to reduce clutter.If it's genuinely unused, apply this diff:
- const isRelative = (id: string) => id.startsWith('.') || id === '.'; -Alternatively, if it was intended for use in the package detection logic, consider using it on line 207:
- if (component.importId && !component.importId.startsWith('.') && storyFilePath) { + if (component.importId && !isRelative(component.importId) && storyFilePath) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/componentManifest/getComponentImports.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
code/renderers/react/src/componentManifest/utils.ts (1)
cachedResolveImport(93-93)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)getImportTag(297-301)
🪛 Biome (2.1.2)
code/renderers/react/src/componentManifest/getComponentImports.ts
[error] 222-222: expected : but instead found {
Remove {
(parse)
[error] 228-228: expected , but instead found ;
Remove ;
(parse)
[error] 231-231: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 233-233: expected , but instead found return
Remove return
(parse)
[error] 229-230: This code will never be reached ...
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
19-19: LGTM: Type definition updated correctly.The addition of the required
isPackagefield toComponentRefensures all components explicitly track their package status.
304-308: Import rewriting logic depends on fragile package detection.The condition
packageName && !c.isPackagecorrectly prevents rewriting actual package imports, but its reliability is limited by the fragilenode_modulesdetection implemented in lines 206-215. IncorrectisPackagevalues could cause:
- Package imports to be rewritten when they shouldn't be (false negatives)
- Relative/tsconfig-path imports to remain unrewritten (false positives)
This is a dependency on the earlier issue rather than a new problem, but be aware that improving the package detection will be critical for correct import rewriting behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
297-300: Fix typo in comment.Line 297: "it the import" should be "if the import".
- : // only rewrite to the package name it the import id is not already a valid package + : // only rewrite to the package name if the import id is not already a valid package // tsconfig paths such as ~/components/Button and components/Button are not seen as packages packageName && !c.isPackage
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/componentManifest/getComponentImports.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
code/renderers/react/src/componentManifest/utils.ts (1)
cachedResolveImport(93-93)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)getImportTag(297-301)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
19-19: LGTM: Type definition correctly extended.The
isPackagefield is appropriately typed as required (non-optional) since it's always initialized in the implementation.
217-226: LGTM: componentWithPackage correctly threaded through.The
componentWithPackageobject (which includesisPackage) is correctly passed togetReactDocgenand returned in both the path-resolved and fallback cases.
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR improves package detection logic for React component imports by introducing an isPackage boolean flag to better distinguish third-party packages from local components and tsconfig path aliases. The change simplifies the codebase by replacing regex-based package name validation with a more reliable resolution-based approach.
Key changes:
- Adds
isPackageflag toComponentReftype that indicates whether an import can be resolved as a package fromnode_modules - Removes the complex
valid-package-name.tsutility and its validation logic - Updates import rewriting logic to use the
isPackageflag instead of package name pattern matching
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.setup.ts |
Updated mock to include package imports that can be resolved, enabling proper testing of package detection |
valid-package-name.ts |
Removed file containing package name validation utilities |
getComponentImports.ts |
Added isPackage detection logic and updated import rewriting to use the flag |
getComponentImports.test.ts |
Updated test expectations to include isPackage flag and adjusted test data |
fixtures.ts |
Cleaned up duplicate @import JSDoc tags in test fixtures |
Comments suppressed due to low confidence (3)
code/renderers/react/src/componentManifest/getComponentImports.test.ts:630
- The test name is misleading. The test actually verifies that the scoped package subpath
@scope/ui/componentsIS rewritten topkg(as shown in line 651). Consider renaming to something like "Rewrites scoped package subpath to packageName" to accurately reflect the behavior.
test('Does not rewrite scoped package subpath (valid bare specifier)', () => {
code/renderers/react/src/componentManifest/getComponentImports.test.ts:658
- The test name is misleading. The test actually verifies that the unscoped package subpath
ui/componentsIS rewritten topkg(as shown in line 679). Consider renaming to something like "Rewrites unscoped package subpath to packageName" to accurately reflect the behavior.
test('Does not rewrite unscoped package subpath (valid bare specifier)', () => {
code/renderers/react/src/componentManifest/getComponentImports.test.ts:623
- Inconsistent quote escaping in the test expectation. Should use escaped double quotes to match the pattern used elsewhere in the test file.
"import { Btn } from "my-package";",
code/renderers/react/src/componentManifest/getComponentImports.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (2)
11-21: ClarifyisPackagedetection semantics and interaction withpackageNamerewrites.The resolver-based
isPackageflag and its use ingetImportsmake the logic much clearer, but there are a couple of edge cases worth tightening:
isPackageonly becomestruewhenstoryFilePathis provided (lines 193‑212). If any caller passes apackageNameintogetImports/getComponentDatawithout also passingstoryFilePath, every component will haveisPackage === false, sogetImportswill rewrite all imports — including real packages like'react'— topackageName(lines 295‑299). That’s a behavior change from the pre‑isPackagelogic and could emit invalid imports. Either:
- guarantee (and document) that
storyFilePathis always supplied wheneverpackageNameis non‑empty, or- add a fallback heuristic for the
isPackagedefault whenstoryFilePathis undefined.The non‑relative check
!component.importId.startsWith('.')(line 207) will treat imports like'/foo/bar'as packages and doesn’t handle the bare'.'case. A small helper such asconst isRelativeImport = (id: string) => id.startsWith('.') || id.startsWith('/');and then checking
!isRelativeImport(component.importId)here would make the intent clearer and avoid those edge cases, aligning with earlier feedback about using anisRelative‑style helper.If current call sites never pass
packageNamewithoutstoryFilePathand never use absolute-path specifiers, the implementation is fine, but codifying those assumptions or adding tests around them would make this more robust.#!/bin/bash # Inspect call sites to ensure packageName isn't used without storyFilePath rg -n "getComponentData\(" -C3 rg -n "getImports\(" -C3 codeAlso applies to: 193-227, 295-299
206-212: Empty catch in package-resolution block hides useful debugging signal.The
catch {}around the secondcachedResolveImportsilently swallows all errors, including unexpected resolver failures (e.g., from alternative module resolution setups). Even though failures are expected for tsconfig-path aliases you don’t want to treat as packages, it would help future debugging to either log at debug level or add a short explanatory comment. Sinceloggeris already imported and used above, mirroring that pattern here would keep behavior consistent.Example:
- } catch {} + } catch (e) { + logger.debug(e); + }
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
295-297: Fix minor typo in comment.Nit: the new comment has a small typo.
- : // only rewrite to the package name it the import id is not already a valid package + : // only rewrite to the package name if the import id is not already a valid package
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/renderers/react/src/componentManifest/getComponentImports.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use camelCase for variable and function names
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
**/*.{ts,tsx,js,jsx,json,html,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
code/**/!(*.test).{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/getComponentImports.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-09-17T07:32:14.512Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/src/core-server/utils/open-in-browser.ts:6-6
Timestamp: 2025-09-17T07:32:14.512Z
Learning: When reviewing async functions that perform side effects like opening browsers, `.catch()` is preferred over `await` if the operation should be non-blocking. This prevents unhandled Promise rejections while keeping the operation as fire-and-forget.
Applied to files:
code/renderers/react/src/componentManifest/getComponentImports.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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 181 KB | 414 KB | 🚨 +232 KB 🚨 |
| Dependency size | 2.97 MB | 2.97 MB | 🚨 +6 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 6 | 🚨 +4 🚨 |
| Self size | 376 KB | 441 KB | 🚨 +66 KB 🚨 |
| Dependency size | 338 KB | 570 KB | 🚨 +232 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 11 | 🎉 -6 🎉 |
| Self size | 304 KB | 291 KB | 🎉 -13 KB 🎉 |
| Dependency size | 2.00 MB | 1.30 MB | 🎉 -701 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 191 | 187 | 🎉 -4 🎉 |
| Self size | 75 KB | 66 KB | 🎉 -9 KB 🎉 |
| Dependency size | 32.19 MB | 31.98 MB | 🎉 -215 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 39 | 44 | 🚨 +5 🚨 |
| Self size | 19.63 MB | 21.70 MB | 🚨 +2.07 MB 🚨 |
| Dependency size | 16.40 MB | 17.16 MB | 🚨 +755 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 191 | 187 | 🎉 -4 🎉 |
| Self size | 118 KB | 114 KB | 🎉 -3 KB 🎉 |
| Dependency size | 30.31 MB | 30.00 MB | 🎉 -317 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 195 | 191 | 🎉 -4 🎉 |
| Self size | 15 KB | 15 KB | 🎉 -30 B 🎉 |
| Dependency size | 28.91 MB | 28.69 MB | 🎉 -224 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 14 | 🎉 -6 🎉 |
| Self size | 22 KB | 22 KB | 🚨 +8 B 🚨 |
| Dependency size | 2.34 MB | 1.62 MB | 🎉 -715 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 537 | 533 | 🎉 -4 🎉 |
| Self size | 645 KB | 645 KB | 🎉 -225 B 🎉 |
| Dependency size | 59.13 MB | 58.91 MB | 🎉 -224 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 128 | 124 | 🎉 -4 🎉 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -111 B 🎉 |
| Dependency size | 21.95 MB | 21.72 MB | 🎉 -229 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 14 | 🎉 -6 🎉 |
| Self size | 13 KB | 13 KB | 🚨 +8 B 🚨 |
| Dependency size | 2.33 MB | 1.61 MB | 🎉 -715 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 160 | 156 | 🎉 -4 🎉 |
| Self size | 30 KB | 30 KB | 🚨 +10 B 🚨 |
| Dependency size | 23.13 MB | 22.90 MB | 🎉 -229 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 118 | 114 | 🎉 -4 🎉 |
| Self size | 35 KB | 35 KB | 🎉 -122 B 🎉 |
| Dependency size | 19.75 MB | 19.52 MB | 🎉 -229 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 277 | 273 | 🎉 -4 🎉 |
| Self size | 24 KB | 24 KB | 🚨 +10 B 🚨 |
| Dependency size | 44.08 MB | 43.85 MB | 🎉 -224 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 199 | 🎉 -4 🎉 |
| Self size | 16 KB | 16 KB | 🚨 +22 B 🚨 |
| Dependency size | 33.45 MB | 33.23 MB | 🎉 -224 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 24 | 19 | 🎉 -5 🎉 |
| Self size | 56 KB | 56 KB | 🚨 +8 B 🚨 |
| Dependency size | 27.00 MB | 26.74 MB | 🎉 -255 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 25 | 20 | 🎉 -5 🎉 |
| Self size | 56 KB | 56 KB | 🚨 +10 B 🚨 |
| Dependency size | 27.06 MB | 26.80 MB | 🎉 -255 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 114 | 109 | 🎉 -5 🎉 |
| Self size | 35 KB | 35 KB | 🎉 -16 B 🎉 |
| Dependency size | 44.15 MB | 43.90 MB | 🎉 -255 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 21 | 15 | 🎉 -6 🎉 |
| Self size | 19 KB | 19 KB | 🚨 +8 B 🚨 |
| Dependency size | 2.37 MB | 1.65 MB | 🎉 -715 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 173 | 188 | 🚨 +15 🚨 |
| Self size | 773 KB | 840 KB | 🚨 +67 KB 🚨 |
| Dependency size | 66.60 MB | 71.38 MB | 🚨 +4.78 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 166 | 170 | 🚨 +4 🚨 |
| Self size | 30 KB | 30 KB | 🚨 +29 B 🚨 |
| Dependency size | 65.17 MB | 67.91 MB | 🚨 +2.73 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 40 | 45 | 🚨 +5 🚨 |
| Self size | 999 KB | 1.45 MB | 🚨 +453 KB 🚨 |
| Dependency size | 36.04 MB | 38.86 MB | 🚨 +2.82 MB 🚨 |
| Bundle Size Analyzer | node | node |
Closes #
What I did
This PR improves package detection logic for React component imports by introducing an isPackage boolean flag to better distinguish third-party packages from local components and tsconfig path aliases. The change simplifies the codebase by replacing regex-based package name validation with a more reliable resolution-based approach.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
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 pull request has been released as version
0.0.0-pr-33090-sha-f6174c82. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-33090-sha-f6174c82kasper/nextf6174c821763555037)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=33090Summary by CodeRabbit
Refactor
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.