Babel v8 upgrade and eslint warning clean-up - #2100
Conversation
|
|
❗ This is currently blocked on |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
WalkthroughUpgrades Babel devDependencies from v7 to v8 with updated plugin names and preset-env configuration. Removes ChangesBabel v8 Upgrade and Dependencies
Code Coverage Removal and Directive Cleanup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
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)
babel.config.js (1)
48-48:⚠️ Potential issue | 🟠 MajorRemove
loose: truefrom thedevenvironment preset—it's a Babel 8 zombie option.Line 48 uses
loose: truein thedevenvironment, but this option was removed in Babel 8 in favor of top-levelassumptions. Thedevconfig is also inconsistent withes5andesm, which both correctly use theassumptionsandexcludepatterns. All the pieces are already there—just need to align it.Suggested fix
dev: { presets: [ - ['`@babel/preset-env`', { targets: { esmodules: true }, loose: true }], + ['`@babel/preset-env`', { targets: { esmodules: true }, exclude }], ], + assumptions, plugins, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@babel.config.js` at line 48, The `dev` environment preset at line 48 includes `loose: true`, which is a deprecated Babel 8 option that was removed in favor of top-level `assumptions`. Remove the `loose: true` property from the `@babel/preset-env` preset configuration in the `dev` environment, and align it with the existing patterns in the `es5` and `esm` environment configurations by using the appropriate `assumptions` and `exclude` properties instead to maintain consistency across all environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 55-60: The Babel 8 packages in package.json require Node ^22.18.0
or >=24.11.0, but the release workflow in .github/workflows/release.yml is
pinned to Node 20, which will cause the release CI to fail when running npm run
test. To fix this, update the Node version in the release.yml workflow to Node
22 or latest to match the test CI configuration, and add an engines field to
package.json specifying the required Node version (e.g., "engines": { "node":
"^22.18.0" }) to prevent incompatible installations and alert contributors
early.
---
Outside diff comments:
In `@babel.config.js`:
- Line 48: The `dev` environment preset at line 48 includes `loose: true`, which
is a deprecated Babel 8 option that was removed in favor of top-level
`assumptions`. Remove the `loose: true` property from the `@babel/preset-env`
preset configuration in the `dev` environment, and align it with the existing
patterns in the `es5` and `esm` environment configurations by using the
appropriate `assumptions` and `exclude` properties instead to maintain
consistency across all environments.
🪄 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: 5677f8c6-ee02-4b98-9481-3a702f584526
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.prettierrc.jsbabel.config.jseslint.config.mjsindex.d.tspackage.jsonrollup.config.mjstest/debug.jstest/fixtures/fixtures.js
💤 Files with no reviewable changes (6)
- .prettierrc.js
- eslint.config.mjs
- test/fixtures/fixtures.js
- test/debug.js
- rollup.config.mjs
- index.d.ts
| "@babel/core": "^8.0.0", | ||
| "@babel/eslint-parser": "^8.0.0", | ||
| "@babel/eslint-plugin": "^8.0.0", | ||
| "@babel/plugin-transform-nullish-coalescing-operator": "^8.0.1", | ||
| "@babel/plugin-transform-optional-chaining": "^8.0.1", | ||
| "@babel/preset-env": "^8.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Node/toolchain declarations found in repo =="
fd -HI '(^package\.json$|^\.nvmrc$|^\.node-version$|^\.tool-versions$)' | while read -r f; do
echo "--- ${f}"
sed -n '1,160p' "${f}"
done
echo
echo "== CI workflow Node settings =="
rg -n "setup-node|node-version|matrix" -g "*.yml" -g "*.yaml"
echo
echo "== Babel ESLint package engine constraints from lockfile =="
if fd -HI '^package-lock\.json$' >/dev/null; then
rg -n -C3 '"node_modules/@babel/eslint-parser"|"node_modules/@babel/eslint-plugin"|"engines"|"node": "\\^22\\.18\\.0 \\|\\| >=24\\.11\\.0"' package-lock.json
else
echo "No package-lock.json found."
fiRepository: focus-trap/tabbable
Length of output: 4194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Looking for CI/workflow files =="
fd -HI '\.(yml|yaml)$' -p '.github' 2>/dev/null || echo "No .github workflows found"
echo
echo "== Node version declaration files =="
fd -HI '^(\.nvmrc|\.node-version|\.tool-versions)$'
echo
echo "== Checking package-lock.json for Babel ESLint package engines =="
if [ -f package-lock.json ]; then
echo "--- `@babel/eslint-parser` engines ---"
jq '.packages["node_modules/@babel/eslint-parser"] | {version, engines}' package-lock.json 2>/dev/null || rg -A 5 '"node_modules/@babel/eslint-parser"' package-lock.json | head -20
echo
echo "--- `@babel/eslint-plugin` engines ---"
jq '.packages["node_modules/@babel/eslint-plugin"] | {version, engines}' package-lock.json 2>/dev/null || rg -A 5 '"node_modules/@babel/eslint-plugin"' package-lock.json | head -20
else
echo "No package-lock.json found"
fi
echo
echo "== Current package.json engines (if any) =="
jq '.engines // "No engines field"' package.jsonRepository: focus-trap/tabbable
Length of output: 583
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CI workflow Node configuration =="
cat -n .github/workflows/ci.yml
echo
echo "== Release workflow Node configuration =="
cat -n .github/workflows/release.ymlRepository: focus-trap/tabbable
Length of output: 4435
Release workflow will fail with Babel 8 packages—Node 20 is incompatible.
The release job (.github/workflows/release.yml, line 35) pins Node to 20, but Babel 8 packages require ^22.18.0 || >=24.11.0. When release CI runs npm run test, the lint step will fail before anything ships.
Additionally, package.json lacks an engines.node declaration. This means installs on Node <22 appear fine locally until CI blows up—a nasty surprise for contributors.
Fixes needed:
- Update
release.ymlto use Node22orlatest(matching the test CI) - Add
"engines": { "node": "^22.18.0" }topackage.jsonto prevent installs on incompatible versions
Note: The test CI already uses latest with check-latest: true, so main CI passes. But that's hiding the release job's incompatibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 55 - 60, The Babel 8 packages in package.json
require Node ^22.18.0 or >=24.11.0, but the release workflow in
.github/workflows/release.yml is pinned to Node 20, which will cause the release
CI to fail when running npm run test. To fix this, update the Node version in
the release.yml workflow to Node 22 or latest to match the test CI
configuration, and add an engines field to package.json specifying the required
Node version (e.g., "engines": { "node": "^22.18.0" }) to prevent incompatible
installations and alert contributors early.
1 participant
PR Checklist
Please leave this checklist in your PR.
typeof document/window !== 'undefined'before using it in code that gets executed on load.npm run changesetlocally to add one, and follow the prompts).Summary by CodeRabbit
Release Notes