chore: update jest version to 29#37587
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Jest configuration, dependency versions in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
github-actions
bot
added
Task
ayushpahwa
requested review from
KelvinOm,
riodeuno and
znamenskii-ilia
and removed request for
KelvinOm
![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
🧹 Outside diff range and nitpick comments (2)
app/client/test/setup.ts (2)
89-89: Use jest.fn() for consistencyOther DOM method mocks use jest.fn(). Consider using the same pattern here.
-Element.prototype.scrollTo = () => { }; +Element.prototype.scrollTo = jest.fn();
96-97: Remove unnecessary eslint disable commentThe eslint disable comment is redundant for an empty arrow function.
- // eslint-disable-next-line @typescript-eslint/no-empty-function - this.onmessage = () => { }; + this.onmessage = () => {};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
app/client/src/components/propertyControls/__snapshots__/IconSelectControl.test.tsx.snapis excluded by!**/*.snapapp/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
app/client/jest.config.js(2 hunks)app/client/package.json(3 hunks)app/client/test/setup.ts(3 hunks)
🧰 Additional context used
🪛 Biome
app/client/test/setup.ts
[error] 7-8: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
app/client/package.json (2)
263-263: LGTM: Jest version updates are consistent
The Jest-related package versions are properly synchronized to v29.x, which aligns with the PR objectives.
Also applies to: 332-332, 334-334
357-357: Verify ts-jest configuration changes
The ts-jest version update requires moving from globals to transform block as mentioned in the PR objectives.
Run this script to verify the ts-jest configuration:
✅ Verification successful
ts-jest configuration is already properly set up
The ts-jest configuration is correctly set up in all Jest config files using the transform block rather than globals. The main config and all package configs are using the modern configuration approach. The only ts-jest settings in globals are for specific options like isolatedModules, which is the correct usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ts-jest is properly configured in transform block
# Look for jest config files
fd -e js -e json -e ts "jest.config"
# Check ts-jest configuration in found files
rg -A 10 "transform.*ts-jest"
Length of output: 586
Script:
#!/bin/bash
# Let's check the content of jest config files to see the ts-jest configuration
for config in $(fd -e js -e json -e ts "jest.config"); do
echo "=== Content of $config ==="
cat "$config"
echo -e "\n"
done
Length of output: 13463
app/client/jest.config.js (1)
71-71: New moduleNameMapper entry for 'entities/*' looks good
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/packages/ast/src/jsObject/index.ts (1)
Line range hint
165-165: Consider using a more specific type instead ofanyThe
anytype reduces type safety. Consider using theNodetype from acorn or a more specific type that matches the expected structure of an export declaration node.Example improvement:
- ExportDefaultDeclaration(node: any) { + ExportDefaultDeclaration(node: Node & { declaration?: { properties: Property[] } }) {app/client/packages/ast/src/actionCreator/index.ts (1)
Test coverage needs to be added for findNodeAt and getQueryParam functions
The test file shows good coverage for most AST manipulation functions, but lacks tests for:
findNodeAtutility functiongetQueryParamfunctionPlease add test cases for these functions to maintain test coverage after the Jest upgrade.
🔗 Analysis chain
Line range hint
239-1429: Verify test coverage for AST manipulation functions.While the TypeScript improvements look good, let's ensure the test coverage remains intact after the Jest upgrade.
Let me check the test coverage for these AST manipulation functions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if tests exist for the modified AST functions # Look for test files that cover these functions fd -e test.ts -e spec.ts . | grep -i "actionCreator" # Search for specific test cases rg -l "findNodeAt|getQueryParam" -g "*.test.ts" -g "*.spec.ts"Length of output: 387
Script:
#!/bin/bash # Check test content for AST manipulation functions cat app/client/packages/ast/src/actionCreator/index.test.ts # Check if there are any test cases for findNodeAt and getQueryParam rg "findNodeAt|getQueryParam" -A 5 -B 5 app/client/packages/ast/src/actionCreator/index.test.tsLength of output: 12545
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/packages/ast/src/actionCreator/index.ts(8 hunks)app/client/packages/ast/src/jsObject/index.ts(1 hunks)
🔇 Additional comments (5)
app/client/packages/ast/src/jsObject/index.ts (1)
89-89: LGTM: Type annotation improves type safety
The explicit Node type annotation from acorn enhances type checking for the AST visitor function.
app/client/packages/ast/src/actionCreator/index.ts (4)
239-239: LGTM: Unused parameter properly marked.
The type parameter is renamed to _type to indicate it's intentionally unused in the callback.
Line range hint 720-738: LGTM: Type annotations added to CallExpression visitor.
The type annotation for the node parameter improves type safety in the AST traversal.
Line range hint 987-1017: LGTM: Consistent handling of unused parameters.
All findNodeAt callback functions consistently mark the unused type parameter with an underscore prefix.
Also applies to: 1018-1070, 1071-1114, 1115-1134, 1391-1406
1429-1429: LGTM: Unused parameter properly marked.
The number parameter is renamed to _number to indicate it's intentionally unused in the function.
3 participants
Description
Update version of the jest package to leverage new node features. Changes needed to make the new version compatible
ts-jesthad to be moved from globals to the transform block since it is a transformer and with new version, it is deprecated to keep them in globals.Arraybefore tests is no longer needed, it is implicit.global.cryptois immutable and hence needs to be deleted before the new lib can be properly used. ReferenceFixes #37586
Automation
/test sanity
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11946938998
Commit: 99c7f2b
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 21 Nov 2024 08:25:00 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
New Features
Chores