test: allow transforming @octokit and ESM deps in Jest; update deps to reproduce octokit upgrade #4503
Conversation
…o reproduce octokit upgrade
Anurag8484
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
WalkthroughUpdates to Jest configuration and package dependencies: added transformIgnorePatterns in jest.config.js and upgraded/added several runtime and dev dependencies (notably @octokit/graphql → 8.x, added @octokit/request v9, i18next/react-i18next, Babel toolchain and babel-jest). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4503--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
package.json (1)
58-59: Consider using semver ranges for consistency.The octokit packages are pinned to exact versions (no caret), while most other dependencies in the project use semver ranges (e.g.,
^7.0.2). This inconsistency can lead to issues with peer dependency resolution and makes it harder to receive patch updates automatically.Apply this diff to use semver ranges:
- "@octokit/graphql": "8.2.1", - "@octokit/request": "9.2.2", + "@octokit/graphql": "^8.2.1", + "@octokit/request": "^9.2.2",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
jest.config.js(1 hunks)package.json(5 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
package.json
[warning] 1-1: EBADENGINE Unsupported engine: package '@tisoap/[email protected]' requires node '>=16' and npm '^8.0.0'; current: node 'v20.11.0', npm '10.2.4'.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (6)
jest.config.js (4)
13-17: LGTM!The multi-line formatting improves readability without changing behavior.
22-22: LGTM!The multi-line formatting is consistent with the rest of the configuration.
3-6: LGTM! The babel config file exists at the specified path.The multi-line formatting improves readability, and the file reference to
./tests/babel.test.config.ctshas been verified.
19-21: Configuration fix is correct; verify tests pass in CI environment.The negative lookahead pattern correctly exempts @octokit, octokit, and universal-user-agent (confirmed as transitive dependency) from Jest's node_modules ignore, allowing babel-jest to transpile them. Babel is properly configured with
babel-plugin-transform-import-metafor ESM support.Confirm that the test suite passes with the upgraded octokit versions in your CI environment, as actual test execution cannot be performed in this sandbox.
package.json (2)
1-1: Pre-existing pipeline warning (not caused by this PR).The pipeline failure for
@tisoap/react-flow-smart-edgeis not related to your changes. However, this dependency requires npm ^8.0.0 but the CI is running npm 10.2.4, which doesn't match the semver range. This should be addressed separately, either by updating the dependency or its engine requirements.Note: This is informational and doesn't block the current PR, but the team should track this separately.
82-82: ****The i18next upgrade is directly related to the PR objective. The commit message explicitly states: "update deps to reproduce octokit upgrade," indicating the dependency updates (including i18next v25.5.2 and new react-i18next v16.1.6) are intentional and necessary for addressing the octokit issue. This is not unrelated scope creep. The codebase actively uses i18next across multiple files, and the upgrades are part of the stated testing and reproduction strategy.
Likely an incorrect or invalid review comment.
| "react-dom": "^18", | ||
| "react-ga": "^3.3.1", | ||
| "react-gtm-module": "^2.0.11", | ||
| "react-i18next": "16.1.6", |
There was a problem hiding this comment.
Unrelated dependency addition increases PR risk.
Similar to the i18next upgrade at line 82, adding react-i18next appears unrelated to fixing octokit tests. This further compounds the scope creep concern.
Consider moving i18n-related changes to a separate PR focused on internationalization updates.
🤖 Prompt for AI Agents
In package.json around line 105, the addition of "react-i18next": "16.1.6" is
unrelated to the octokit test fixes and broadens this PR's scope; remove this
dependency change from the current branch (restore package.json to the version
without react-i18next) and open a separate PR that contains i18n-related
dependency upgrades (including react-i18next and any paired i18next updates),
ensuring package-lock/yarn.lock is updated only in that dedicated PR.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4503 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 778
Branches 144 144
=========================================
Hits 778 778 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
82-82: Scope creep: i18n updates unrelated to octokit test fixes.The i18next (line 82) and react-i18next (line 105) updates appear unrelated to the PR's stated objective of fixing Jest ESM transpilation for @octokit. These changes expand scope unnecessarily.
Are these i18n dependency updates required for the octokit upgrade, or can they be moved to a separate PR? If unrelated, restore package.json to exclude react-i18next and revert i18next to its prior version, ensuring package-lock.json changes reflect only octokit and Babel-related updates.
Also applies to: 105-105
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json(5 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (1)
package.json (1)
128-130: Good: Babel and babel-jest versions corrected and aligned.The Babel packages (@babel/core, @babel/preset-env, @babel/preset-typescript) are now correctly pinned to ^7.28.4 (valid version), and babel-jest is aligned to ^29.7.0 to match Jest ^29.7.0. These corrections support the ESM transpilation strategy in jest.config.js.
Also applies to: 155-155, 171-171
| "@octokit/graphql": "8.2.1", | ||
| "@octokit/request": "9.2.2", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify octokit versions are valid and stable.
Both @octokit/graphql and @octokit/request are pinned to exact versions (no caret). Ensure these versions exist on npm and are stable releases.
🌐 Web query:
What is the latest version of @octokit/graphql and @octokit/request packages?
💡 Result:
As of October 24, 2025:
- @octokit/graphql — 9.0.1. [1]
- @octokit/request — 10.0.5. [2]
Sources:
[1] npm package page for @octokit/graphql.
[2] Yarn/npm registry listing (package versions) for @octokit/request.
Octokit versions verified; flag i18next/react-i18next as scope creep.
The @octokit/graphql 8.2.1 and @octokit/request 9.2.2 versions are valid and exist on npm. However, the changes to i18next (line 82) and the addition of react-i18next (line 105) appear unrelated to the octokit ESM transpilation support objective—these belong in a separate PR and should be reverted to keep scope focused.
🤖 Prompt for AI Agents
In package.json around lines 58-59, the @octokit/graphql and @octokit/request
version changes are fine but unrelated edits to i18next (around line 82) and the
new react-i18next entry (around line 105) are scope creep; revert the i18next
change to its prior version and remove the react-i18next addition so only the
octokit dependency updates remain, then run npm/yarn install to regenerate the
lockfile.
|
The build is failing on Netlify due to a missing |
2 participants
Closes #3690
Updated jest.config.js to include transformIgnorePatterns so babel-jest transpiles @octokit and its ESM deps. This fixes Jest "import/export" parse errors when upgrading Octokit.
Summary by CodeRabbit
Chores
New Features
Tests