-
-
Notifications
You must be signed in to change notification settings - Fork 4
fix: revert standard_only_visit_mut to noop_visit_mut_type
#373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Fixes testing with testing-library with jsx/tsx
WalkthroughReplaces a VisitMut macro in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)tests/integration/jest.config.cjs(2 hunks)tests/integration/src/jsx.test.jsx(1 hunks)tests/integration/src/tsx.test.tsx(1 hunks)tests/integration/src/typescript.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/src/tsx.test.tsx (1)
tests/integration/src/jsx.test.jsx (2)
TestComponent(4-8)undefinedValue(5-5)
⏰ 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: Test - windows-latest
- GitHub Check: Test - ubuntu-latest
🔇 Additional comments (2)
tests/integration/src/typescript.test.ts (1)
1-3: LGTM!Converting from an ambient declaration to a concrete interface is appropriate for this test file and aligns with the pattern used in the new JSX/TSX integration tests.
tests/integration/jest.config.cjs (1)
2-84: LGTM!The Jest configuration correctly extends support for JSX and TSX files with appropriate parser settings for each file type. The per-extension transform blocks properly configure the SWC plugin, and the
@happy-dom/jest-environmentis well-suited for React component testing.
| "@happy-dom/jest-environment": "^20.0.11", | ||
| "@swc/core": "^1.15.4", | ||
| "@swc/jest": "^0.2.39", | ||
| "@testing-library/react": "^16.3.0", | ||
| "@types/jest": "^30.0.0", | ||
| "@types/react": "^19.2.7", | ||
| "auto-changelog": "^2.5.0", | ||
| "jest": "^30.2.0", | ||
| "react": "^19.2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify package versions exist on npm and check for vulnerabilities
echo "=== Checking if package versions exist on npm ==="
packages=(
"@happy-dom/[email protected]"
"@swc/[email protected]"
"@swc/[email protected]"
"@testing-library/[email protected]"
"@types/[email protected]"
"@types/[email protected]"
"[email protected]"
"[email protected]"
"[email protected]"
)
for pkg in "${packages[@]}"; do
version=$(npm view "$pkg" version 2>&1)
if [ $? -eq 0 ]; then
echo "✓ $pkg exists"
else
echo "✗ $pkg - ERROR or does not exist: $version"
fi
done
echo ""
echo "=== Checking npm audit for vulnerabilities ==="
# Create a temporary package.json to test
temp_dir=$(mktemp -d)
cat > "$temp_dir/package.json" << 'EOF'
{
"name": "temp-audit",
"version": "1.0.0",
"dependencies": {
"@happy-dom/jest-environment": "^20.0.11",
"@swc/core": "^1.15.4",
"@swc/jest": "^0.2.39",
"@testing-library/react": "^16.3.0",
"@types/jest": "^30.0.0",
"@types/react": "^19.2.7",
"auto-changelog": "^2.5.0",
"jest": "^30.2.0",
"react": "^19.2.3"
}
}
EOF
# Run npm audit on the temporary package.json (don't install, just check metadata)
cd "$temp_dir"
npm audit --json 2>/dev/null | python3 << 'PYEOF'
import json
import sys
try:
data = json.load(sys.stdin)
if 'vulnerabilities' in data:
vulns = data['vulnerabilities']
if vulns:
print(f"Found {len(vulns)} vulnerable packages:")
for pkg_name, details in vulns.items():
print(f"\n Package: {pkg_name}")
if 'via' in details:
for via in details['via']:
if isinstance(via, dict):
print(f" - {via.get('title', 'Unknown')}")
print(f" Severity: {via.get('severity', 'Unknown')}")
else:
print("No vulnerabilities found")
else:
print("Audit data structure unexpected")
except Exception as e:
print(f"Error parsing audit data: {e}")
PYEOF
rm -rf "$temp_dir"Repository: Codex-/swc-plugin-import-meta-env
Length of output: 163
🏁 Script executed:
#!/bin/bash
# Simpler version check using npm view with timeout and fallback
echo "=== Quick npm version checks (with timeout) ==="
for pkg in react jest "@testing-library/react" "@types/react"; do
timeout 10 npm view "$pkg@latest" version 2>/dev/null || echo "Could not fetch latest version for $pkg"
done
echo ""
echo "=== Checking package.json syntax ==="
if command -v jq &> /dev/null; then
cd /tmp && cat > package.json << 'EOF'
{
"dependencies": {
"@happy-dom/jest-environment": "^20.0.11",
"@swc/core": "^1.15.4",
"@swc/jest": "^0.2.39",
"@testing-library/react": "^16.3.0",
"@types/jest": "^30.0.0",
"@types/react": "^19.2.7",
"auto-changelog": "^2.5.0",
"jest": "^30.2.0",
"react": "^19.2.3"
}
}
EOF
jq '.dependencies' package.json
fiRepository: Codex-/swc-plugin-import-meta-env
Length of output: 618
🌐 Web query:
React 19.2.3 security vulnerabilities CVE
💡 Result:
Short answer: React 19.2.3 is a security fix release that addresses the December 2025 Server Components vulnerabilities (notably CVE-2025-55182 and the follow‑on issues CVE-2025-55184, CVE-2025-55183 and the incomplete‑fix CVE-2025-67779). Upgrade to 19.2.3 (or later) immediately if you use React Server Components / related server DOM packages. [1][2][3]
Sources
- React security advisories (December 3 and December 11, 2025). [1][2]
- NVD entry for CVE-2025-67779 (incomplete‑fix DoS). [3]
- Independent writeups summarizing affected versions and fixed releases. [4]
🌐 Web query:
testing-library react 16.3.0 security advisories
💡 Result:
Short answer: No public security advisories are reported for @testing-library/react version 16.3.0. Snyk and package security listings show "no known security issues" for 16.3.0; some third‑party scanners note generic package warnings but not a specific CVE for this package version. (security.snyk.io)
Citations:
🌐 Web query:
jest 30.2.0 security issues
💡 Result:
Short answer: Yes — Jest 30.2.0 pulls in vulnerable transitive deps.
What’s affected
- js-yaml prototype-pollution (GHSA-mh29-5h37-fv8m / CVE-2025‑64718). js-yaml <4.1.1 (and <3.14.2) is vulnerable; several Jest packages depend on js-yaml 3.14.2 in 30.2.0. [1][2]
- glob CLI command-injection (GHSA-5j98-mcp5-4vw2 / CVE-2025‑64756) affecting the glob CLI (-c/--cmd); glob appears in Jest dependency tree for 30.2.0. [3][4]
Sources:
- js-yaml GHSA advisory (CVE-2025‑64718). [1]
- Jest GitHub issue showing js-yaml present in v30.2.0. [2]
- OSV/GHSA entry for glob CLI command-injection (CVE-2025‑64756). [3]
- Jest GitHub issue reporting glob vulnerability in v30.2.0. [4]
Recommended actions
- Treat these as transitive-dependency issues — upgrade to patched versions when Jest publishes them. (There are open Jest issues; no universal Jest patch referenced yet.) [2][4]
- Short-term mitigations: avoid parsing untrusted YAML in environments using the vulnerable js-yaml; do not run glob CLI with -c/--cmd on untrusted file names or avoid using the glob CLI in CI on untrusted inputs; install Jest only as devDependency and avoid installing dev deps in production builds (npm/pnpm install --production). [1][3][2]
If you want, I can:
- check whether a Jest release/patch has been published addressing these (I’ll look for a specific Jest fix/release note), or
- scan your project’s dependency tree and show exactly where the vulnerable packages come from.
🌐 Web query:
Jest 30.3.0 30.4.0 release notes js-yaml glob vulnerability fix
💡 Result:
Short answer: Jest 30.3.0 / 30.4.0 do not contain published fixes — the js-yaml and glob vulnerabilities were reported against Jest 30.x and issues were opened asking maintainers to update dependencies, but as of the latest Jest releases/tags the project had not shipped a 30.3.0/30.4.0 release that resolves them.
Sources:
- GitHub Advisory for js-yaml prototype-pollution (CVE / GHSA) — patched in js-yaml 4.1.1 / 3.14.2. [1]
- Jest issue for js-yaml vulnerability (opened Nov 18, 2025). [2]
- Jest issue requesting glob update to support [email protected]+ for the security fix (opened Nov 20, 2025). [3]
- Jest releases / version listing (latest stable shown as 30.0 and npm release history showing latest published 30.2.0 as of Sep 28, 2025). [4][5]
If you want, I can:
- watch the Jest repo and notify you when a release explicitly lists upgrades to js-yaml (>=4.1.1 / 3.14.2) and glob (>=10.5.0), or
- suggest temporary mitigation steps (pinning/overriding dependency versions via npm/yarn/pnpm).
References
- GitHub Advisory — js-yaml prototype pollution (patched in 4.1.1 / 3.14.2). [https://github.com/advisories/GHSA-mh29-5h37-fv8m]
- Issue: "js-yaml medium vulnerability CVE-2025-64718" — jestjs/jest #15893. [https://github.com/[Bug]: js-yaml medium vulnerability CVE-2025-64718 jestjs/jest#15893]
- Issue: "Update dependencies to support [email protected]+ for security fix" — jestjs/jest #15910. [https://github.com/Update dependencies to support [email protected]+ for security fix jestjs/jest#15910]
- Issue: "glob HIGH vulnerability - CVE-2025-64756" — jestjs/jest #15898. [https://github.com/[Bug]: glob HIGH vulnerability - CVE-2025-64756 jestjs/jest#15898]
- Jest releases / versions (GitHub releases + npm release history). [https://github.com/jestjs/jest/releases], [https://releasealert.dev/npmjs/_/jest]
Jest 30.2.0 contains unresolved transitive dependency vulnerabilities that should be mitigated or monitored.
React 19.2.3 is the correct patched version addressing December 2025 Server Components vulnerabilities (CVE-2025-55182 and related CVEs). @testing-library/react 16.3.0 is free from known security advisories.
However, Jest 30.2.0 pulls in vulnerable transitive dependencies:
js-yaml <4.1.1is vulnerable to prototype-pollution (CVE-2025-64718)glob <10.5.0is vulnerable to command-injection (CVE-2025-64756)
Since Jest is a devDependency and these are transitive issues, the risk is mitigated. Nevertheless, consider monitoring Jest for a patched release (issues #15893 and #15898 are open on jestjs/jest) or use dependency override tools (npm overrides, yarn resolutions) to pin safer transitive versions as a short-term measure.
🤖 Prompt for AI Agents
package.json around lines 38 to 46: the pinned devDependency [email protected] pulls
vulnerable transitive versions of js-yaml and glob; update your remediation by
either (1) adding dependency overrides/resolutions (npm overrides or yarn
resolutions) to force js-yaml to >=4.1.1 and glob to >=10.5.0, or (2) upgrade
jest when a patched release is available and document this as monitored risk in
the repo; ensure these changes are limited to devDependencies and add a short
comment in package.json or README noting the mitigation and that the issue is
tracked against jestjs/jest issues #15893 and #15898.
| return <div data-testid="app">{undefinedValue}</div>; | ||
| }; | ||
|
|
||
| describe("tsx", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix test suite label to match file type.
The describe block is labeled "tsx" but this is a JSX file. The label should be "jsx" for consistency.
Apply this diff:
-describe("tsx", () => {
+describe("jsx", () => {📝 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.
| describe("tsx", () => { | |
| describe("jsx", () => { |
🤖 Prompt for AI Agents
tests/integration/src/jsx.test.jsx around line 10: the describe block is labeled
"tsx" but the file is JSX; update the test suite label to "jsx" so it matches
the file type by replacing the "tsx" string in the describe(...) call with
"jsx".
| it("should render a component using import.meta.env", () => { | ||
| render(<TestComponent />); | ||
| const element = screen.getByTestId("app"); | ||
| expect(element).not.toBe(true); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix meaningless assertion.
The assertion expect(element).not.toBe(true) will always pass because element is an HTMLElement, which can never be strictly equal to the boolean true. This test doesn't verify that the component renders correctly or that import.meta.env works as expected.
Apply this diff to test the actual rendered content:
it("should render a component using import.meta.env", () => {
render(<TestComponent />);
const element = screen.getByTestId("app");
- expect(element).not.toBe(true);
+ expect(element.textContent).toBe("undefined");
});This verifies that typeof import.meta.env.attempt correctly evaluates to "undefined" and is rendered in the component.
📝 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.
| it("should render a component using import.meta.env", () => { | |
| render(<TestComponent />); | |
| const element = screen.getByTestId("app"); | |
| expect(element).not.toBe(true); | |
| }); | |
| it("should render a component using import.meta.env", () => { | |
| render(<TestComponent />); | |
| const element = screen.getByTestId("app"); | |
| expect(element.textContent).toBe("undefined"); | |
| }); |
🤖 Prompt for AI Agents
In tests/integration/src/jsx.test.jsx around lines 15-19, the assertion
expect(element).not.toBe(true) is meaningless because an HTMLElement will never
equal the boolean true; replace it with an assertion that verifies the actual
rendered content: read the element's textContent (or use
screen.getByTestId("app").textContent) and assert that it contains the expected
string showing typeof import.meta.env.attempt is "undefined" (or assert equality
to the exact rendered string your component produces). Ensure the test checks
the component output rather than comparing the element to a boolean.
| it("should render a component using import.meta.env", () => { | ||
| render(<TestComponent />); | ||
| const element = screen.getByTestId("app"); | ||
| expect(element).not.toBe(true); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix meaningless assertion.
The assertion expect(element).not.toBe(true) will always pass because element is an HTMLElement, which can never be strictly equal to the boolean true. This test doesn't verify that the component renders correctly or that import.meta.env works as expected.
Apply this diff to test the actual rendered content:
it("should render a component using import.meta.env", () => {
render(<TestComponent />);
const element = screen.getByTestId("app");
- expect(element).not.toBe(true);
+ expect(element.textContent).toBe("undefined");
});This verifies that typeof import.meta.env.attempt correctly evaluates to "undefined" and is rendered in the component.
📝 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.
| it("should render a component using import.meta.env", () => { | |
| render(<TestComponent />); | |
| const element = screen.getByTestId("app"); | |
| expect(element).not.toBe(true); | |
| }); | |
| it("should render a component using import.meta.env", () => { | |
| render(<TestComponent />); | |
| const element = screen.getByTestId("app"); | |
| expect(element.textContent).toBe("undefined"); | |
| }); |
🤖 Prompt for AI Agents
In tests/integration/src/tsx.test.tsx around lines 19-23, the assertion
`expect(element).not.toBe(true)` is meaningless because an HTMLElement is never
the boolean true; replace it with an assertion that verifies the rendered
content correctly reflects import.meta.env behavior. Change the expectation to
check the element's text content for the string "undefined" (e.g., assert
element.textContent or use toHaveTextContent("undefined")), so the test verifies
that typeof import.meta.env.attempt is rendered as "undefined".
Fixes testing with testing-library with jsx/tsx
Fixes #363
Summary by CodeRabbit
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.