Fix vue-component-meta barrel file substring matching#34522
Fix vue-component-meta barrel file substring matching#34522mspostolov wants to merge 2 commits intostorybookjs:nextfrom
Conversation
📝 WalkthroughWalkthroughAdds a Vitest test suite for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts (1)
125-129:⚠️ Potential issue | 🟠 MajorEscape dynamic names in regex patterns and use proper identifier boundaries instead of
\b.Export names can include
$(valid in JavaScript identifiers), but unescaped regex metacharacters and\bword boundaries fail to match them correctly. For example, an identifier$Componentwon't match the pattern\b$Component\b, causing the binding detection to skip valid local exports and incorrectly omit__docgenInfoinjection.Use
escapeRegExpfromes-toolkit/string(already used elsewhere in the codebase) and replace\bboundaries with a pattern that accounts for$and_as valid identifier characters.Proposed fix
+import { escapeRegExp } from 'es-toolkit/string'; + export async function vueComponentMeta(tsconfigPath = 'tsconfig.json'): Promise<Plugin> { // ... existing code ... metaSources.forEach((meta) => { const isDefaultExport = meta.exportName === 'default'; const name = isDefaultExport ? '_sfc_main' : meta.exportName; + const escapedName = escapeRegExp(name); // we can only add the "__docgenInfo" to variables that are actually defined in the current file // so e.g. re-exports like "export { default as MyComponent } from './MyComponent.vue'" must be ignored // to prevent runtime errors if ( - new RegExp(`export {.*${name}.*}`).test(src) || - new RegExp(`export \\* from ['"]\\S*${name}['"]`).test(src) || + new RegExp(`export {.*${escapedName}.*}`).test(src) || + new RegExp(`export \\* from ['"]\\S*${escapedName}['"]`).test(src) || // when using re-exports, some exports might be resolved via checker.getExportNames // but are not directly exported inside the current file so we need to ignore them too - !new RegExp(`\\b${name}\\b`).test(src) + !new RegExp(`(^|[^$_\\w])${escapedName}(?=[^$_\\w]|$)`).test(src) ) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts` around lines 125 - 129, Escape dynamic export names with escapeRegExp (imported from 'es-toolkit/string') and replace the `\b${name}\b` check with an identifier-aware boundary so identifiers containing `$` or `_` are matched; e.g., compute const escaped = escapeRegExp(name) and use new RegExp(`export {.*${escaped}.*}`) and new RegExp(`export \\* from ['"]\\S*${escaped}['"]`) for the first two checks, and replace `!new RegExp(`\\b${name}\\b`).test(src)` with a negative test using an identifier boundary like `!new RegExp('(?<![A-Za-z0-9_$])' + escaped + '(?![A-Za-z0-9_$])').test(src)` (or the equivalent `(^|[^A-Za-z0-9_$])`/`($|[^A-Za-z0-9_$])` pattern) so names with `$`/`_` are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/frameworks/vue3-vite/src/plugins/vue-component-meta.test.ts`:
- Around line 23-43: The tests currently define vi.mock(...) calls and set mock
behaviors inline in test bodies; update each vi.mock(...) invocation (for
'vue-component-meta', 'storybook/internal/common', 'vue-docgen-api', and
'node:fs/promises') to include the option { spy: true }, then move all per-test
mock behavior wiring into a beforeEach block where you obtain type-safe mock
references using vi.mocked(mockChecker) (and vi.mocked(...) for any other mocked
exports) and configure their behaviors there instead of using direct
.mockReturnValue() calls inside tests; ensure you replace direct mock access in
tests with these vi.mocked(...) references and perform all setup in beforeEach
so tests only assert behavior.
---
Outside diff comments:
In `@code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts`:
- Around line 125-129: Escape dynamic export names with escapeRegExp (imported
from 'es-toolkit/string') and replace the `\b${name}\b` check with an
identifier-aware boundary so identifiers containing `$` or `_` are matched;
e.g., compute const escaped = escapeRegExp(name) and use new RegExp(`export
{.*${escaped}.*}`) and new RegExp(`export \\* from ['"]\\S*${escaped}['"]`) for
the first two checks, and replace `!new RegExp(`\\b${name}\\b`).test(src)` with
a negative test using an identifier boundary like `!new
RegExp('(?<![A-Za-z0-9_$])' + escaped + '(?![A-Za-z0-9_$])').test(src)` (or the
equivalent `(^|[^A-Za-z0-9_$])`/`($|[^A-Za-z0-9_$])` pattern) so names with
`$`/`_` are handled correctly.
🪄 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: e030c1a1-e0cd-4b57-986b-bfdc998d1d0d
📒 Files selected for processing (2)
code/frameworks/vue3-vite/src/plugins/vue-component-meta.test.tscode/frameworks/vue3-vite/src/plugins/vue-component-meta.ts
Use word-boundary regex instead of `src.includes(name)` to prevent injecting `__docgenInfo` for export names that are substrings of barrel re-export paths (e.g. `Tab` matching inside `./Tabs`).
mspostolov
force-pushed
the
fix/vue-component-meta-barrel-substring
branch
from
4bf0ec8 to
63c08df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts`:
- Around line 127-131: The named re-export regex `new RegExp(\`export
{.*${escaped}.*}\`)` is line-bound and misses multiline re-exports; update that
RegExp to allow newlines by either adding the dotAll flag (e.g. `new
RegExp(\`export {.*${escaped}.*}\`, 's')`) or replacing `.*` with `[\s\S]*` so
it matches across line breaks, ensuring the `export { ... } from ...` detection
correctly prevents injecting __docgenInfo into non-local symbols.
🪄 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: ea1b24e7-caf9-4876-980a-cb4c58a5363f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
code/frameworks/vue3-vite/package.jsoncode/frameworks/vue3-vite/src/plugins/vue-component-meta.test.tscode/frameworks/vue3-vite/src/plugins/vue-component-meta.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/frameworks/vue3-vite/src/plugins/vue-component-meta.test.ts
| new RegExp(`export {.*${escaped}.*}`).test(src) || | ||
| new RegExp(`export \\* from ['"]\\S*${escaped}['"]`).test(src) || | ||
| // when using re-exports, some exports might be resolved via checker.getExportNames | ||
| // but are not directly exported inside the current file so we need to ignore them too | ||
| !src.includes(name) | ||
| !new RegExp(`(^|[^$_\\w])${escaped}(?=[^$_\\w]|$)`).test(src) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the multiline re-export edge case against current regex behavior.
python - <<'PY'
import re
name = "Tab"
escaped = re.escape(name)
current_named = re.compile(rf"export {{.*{escaped}.*}}")
current_boundary = re.compile(rf"(^|[^$_\w]){escaped}(?=[^$_\w]|$)")
src = """export {
Tab
} from './Tabs'
"""
print("current_named_matches:", bool(current_named.search(src))) # expected False with current pattern
print("current_boundary_matches:", bool(current_boundary.search(src)))# expected True
print("=> current guard can miss named re-export and still treat token as local")
PYRepository: storybookjs/storybook
Length of output: 197
🏁 Script executed:
# First, check the file exists and read the target lines
head -n 135 code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts | tail -n 20Repository: storybookjs/storybook
Length of output: 1095
🏁 Script executed:
# Get more context around the target lines
head -n 150 code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts | tail -n 30Repository: storybookjs/storybook
Length of output: 1538
🏁 Script executed:
# Verify the exact snippet matches what was provided in the review
sed -n '120,135p' code/frameworks/vue3-vite/src/plugins/vue-component-meta.tsRepository: storybookjs/storybook
Length of output: 943
🏁 Script executed:
python - <<'PY'
import re
# Test cases
test_cases = [
# (name, src, description)
("Tab", """export {
Tab
} from './Tabs'
""", "multiline named re-export"),
("Tab", "export { Tab } from './Tabs'", "single-line named re-export"),
("Tab", "export { Tab as MyTab } from './Tabs'", "named re-export with alias"),
("MyTab", """export {
Tab as MyTab,
Pane
} from './Tabs'
""", "multiline with alias"),
("Tab", "export * from './Tabs'", "wildcard re-export"),
("Tab", """export * from './Tabs'
export { OtherComp } from './Other'
""", "mixed exports"),
("Tab", "const Tab = 5; export { Tab }", "local definition with re-export same file"),
("localTab", "const localTab = 5;", "local variable, no export"),
]
# Proposed patterns from the review
def test_proposed_patterns(name, src):
escaped = re.escape(name)
isNamedReExport = re.compile(
rf"export\s*{{[^}}]*\b{escaped}\b[^}}]*}}\s*from\s*['\"][^'\"]+['\"]",
re.MULTILINE
).search(src) is not None
isWildcardReExport = re.compile(
rf"export\s*\*\s*from\s*['\"][^'\"]*{escaped}[^'\"]*['\"]",
re.MULTILINE
).search(src) is not None
hasIdentifierToken = re.compile(
rf"(^|[^$_\w]){escaped}(?=[^$_\w]|$)"
).search(src) is not None
# Logic: skip if (isNamedReExport OR isWildcardReExport OR !hasIdentifierToken)
should_skip = isNamedReExport or isWildcardReExport or not hasIdentifierToken
return isNamedReExport, isWildcardReExport, hasIdentifierToken, should_skip
print("Testing proposed fix patterns:")
print("-" * 100)
for name, src, desc in test_cases:
named, wildcard, token, skip = test_proposed_patterns(name, src)
print(f"{desc:45} | named={str(named):5} wildcard={str(wildcard):5} token={str(token):5} skip={str(skip):5}")
PYRepository: storybookjs/storybook
Length of output: 980
🏁 Script executed:
python - <<'PY'
import re
# Current broken patterns (from the code)
def test_current_patterns(name, src):
escaped = re.escape(name)
# Current patterns - NO multiline flag
namedReExportCurrent = re.compile(rf"export {{.*{escaped}.*}}").search(src) is not None
wildcardCurrent = re.compile(rf"export \* from ['\"]\\S*{escaped}['\"]").search(src) is not None
hasIdentifierToken = re.compile(rf"(^|[^$_\w]){escaped}(?=[^$_\w]|$)").search(src) is not None
should_skip = namedReExportCurrent or wildcardCurrent or not hasIdentifierToken
return namedReExportCurrent, wildcardCurrent, hasIdentifierToken, should_skip
# Test the critical multiline case
name = "Tab"
src = """export {
Tab
} from './Tabs'
"""
named, wildcard, token, skip = test_current_patterns(name, src)
print("Current (broken) patterns on multiline named re-export:")
print(f" named={named}, wildcard={wildcard}, token={token}")
print(f" should_skip={skip}")
print()
print("Expected: skip=True (it's a re-export, don't inject)")
print("Actual: skip=False (BUG: will incorrectly inject __docgenInfo)")
print()
# Demonstrate the injection would fail
print("This causes: ReferenceError: Tab is not defined")
print("Because the code tries to do: Tab.__docgenInfo = {...}")
print("But Tab is not actually defined in the file (it's imported from './Tabs')")
PYRepository: storybookjs/storybook
Length of output: 477
🏁 Script executed:
python - <<'PY'
import re
def test_proposed(name, src):
escaped = re.escape(name)
isNamedReExport = re.compile(
rf"export\s*{{[^}}]*\b{escaped}\b[^}}]*}}\s*from\s*['\"][^'\"]+['\"]",
re.MULTILINE
).search(src) is not None
return isNamedReExport
# Edge cases with formatting and comments
test_cases = [
("Tab", """export {
// comment
Tab,
Pane
} from './Tabs'
""", "with comment"),
("Tab", """export {
Tab // trailing comment
} from './Tabs'
""", "with trailing comment and tabs"),
("Tab", """export {
Tab
} from "./Tabs" // line comment
""", "quotes and line comment"),
]
print("Testing edge cases with proposed pattern:")
for name, src, desc in test_cases:
result = test_proposed(name, src)
print(f" {desc:40} detected={result}")
PYRepository: storybookjs/storybook
Length of output: 277
Harden re-export detection for multiline export { ... } from ... blocks.
The current regex export {.*${escaped}.*} is line-bound and fails to detect multiline named re-exports. When a re-export spans multiple lines, the pattern returns false, but the boundary token check still succeeds, causing the code to incorrectly inject __docgenInfo into an undefined variable and raising a ReferenceError at runtime.
The proposed fix adds the multiline flag (m) to the named re-export pattern, correctly matching exports across line breaks. All edge cases—including comments, tabs, and various formatting—are handled correctly by the updated patterns.
💡 Proposed fix
- if (
- new RegExp(`export {.*${escaped}.*}`).test(src) ||
- new RegExp(`export \\* from ['"]\\S*${escaped}['"]`).test(src) ||
- // when using re-exports, some exports might be resolved via checker.getExportNames
- // but are not directly exported inside the current file so we need to ignore them too
- !new RegExp(`(^|[^$_\\w])${escaped}(?=[^$_\\w]|$)`).test(src)
- ) {
+ const isNamedReExport = new RegExp(
+ `export\\s*{[^}]*\\b${escaped}\\b[^}]*}\\s*from\\s*['"][^'"]+['"]`,
+ 'm'
+ ).test(src);
+ const isWildcardReExport = new RegExp(
+ `export\\s*\\*\\s*from\\s*['"][^'"]*${escaped}[^'"]*['"]`,
+ 'm'
+ ).test(src);
+ const hasIdentifierToken = new RegExp(
+ `(^|[^$_\\w])${escaped}(?=[^$_\\w]|$)`
+ ).test(src);
+
+ if (isNamedReExport || isWildcardReExport || !hasIdentifierToken) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| new RegExp(`export {.*${escaped}.*}`).test(src) || | |
| new RegExp(`export \\* from ['"]\\S*${escaped}['"]`).test(src) || | |
| // when using re-exports, some exports might be resolved via checker.getExportNames | |
| // but are not directly exported inside the current file so we need to ignore them too | |
| !src.includes(name) | |
| !new RegExp(`(^|[^$_\\w])${escaped}(?=[^$_\\w]|$)`).test(src) | |
| const isNamedReExport = new RegExp( | |
| `export\\s*{[^}]*\\b${escaped}\\b[^}]*}\\s*from\\s*['"][^'"]+['"]`, | |
| 'm' | |
| ).test(src); | |
| const isWildcardReExport = new RegExp( | |
| `export\\s*\\*\\s*from\\s*['"][^'"]*${escaped}[^'"]*['"]`, | |
| 'm' | |
| ).test(src); | |
| const hasIdentifierToken = new RegExp( | |
| `(^|[^$_\\w])${escaped}(?=[^$_\\w]|$)` | |
| ).test(src); | |
| if (isNamedReExport || isWildcardReExport || !hasIdentifierToken) { | |
| return; | |
| } |
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 127-127: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(export \\* from ['"]\\S*${escaped}['"])
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 130-130: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((^|[^$_\\w])${escaped}(?=[^$_\\w]|$))
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts` around lines 127
- 131, The named re-export regex `new RegExp(\`export {.*${escaped}.*}\`)` is
line-bound and misses multiline re-exports; update that RegExp to allow newlines
by either adding the dotAll flag (e.g. `new RegExp(\`export {.*${escaped}.*}\`,
's')`) or replacing `.*` with `[\s\S]*` so it matches across line breaks,
ensuring the `export { ... } from ...` detection correctly prevents injecting
__docgenInfo into non-local symbols.
2 participants
Use word-boundary regex instead of
src.includes(name)to prevent injecting__docgenInfofor export names that are substrings of barrel re-export paths (e.g.Tabmatching inside./Tabs).Closes #34521
What I did
The
vue-component-metaplugin usedsrc.includes(name)to check whether an export name is a local binding. This caused false positives when the name was a substring of a re-export path (e.g.Tabmatched insideexport * from './Tabs'), leading toReferenceError: Tab is not definedat runtime.Replaced
!src.includes(name)with!new RegExp(\\b${name}\b`).test(src)` to use word-boundary matching instead of substring matching.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
npm installandnpm run storybookReferenceError: Tab is not definedin the browser consoleDocumentation
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
Bug Fixes
Tests