Skip to content
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

chore: upgrde jest to v29 #1473

Merged
merged 18 commits into from
Aug 7, 2023
Merged

chore: upgrde jest to v29 #1473

merged 18 commits into from
Aug 7, 2023

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Jul 26, 2023

Dependency Changes

Note
Didn't upgrade @testing-library/react to v13+ since it drops support for React 17 and earlier.

  • Upgraded the following dependencies to their latest version:
    • @testing-library/react-hooks to v8
    • @testing-library/react-native to v12.1.3
    • babel-jest, jest, and jest-environment-jsdom to v29.6.1
    • jest-axe to v8
    • jest-styled-components to v7.1.1
    • react-native-reanimated to v2.14.4, for compatibility with Jest v28+

Jest Config Changes

Source Code Changes

Test Changes

Snapshot Changes

  • As announced in the Jest 28 blog post, Jest 29 has changed the default snapshot formatting to {escapeString: false, printBasicPrototype: false}. This makes snapshots both more readable and more copy-pasteable.

GitHub Action Changes

  • Refactored validate workflow to enable Jest Sharding, which has reduced the total workflow time by 3-4 minutes.

@changeset-bot
Copy link

changeset-bot bot commented Jul 26, 2023

⚠️ No Changeset found

Latest commit: 1885bc2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 26, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1885bc2:

Sandbox Source
razorpay/blade: basic Configuration

@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2023

✅ PR title follows Conventional Commits specification.

@@ -0,0 +1,34 @@
diff --git a/node_modules/react-native-reanimated/lib/reanimated2/jestUtils.js b/node_modules/react-native-reanimated/lib/reanimated2/jestUtils.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this patch? I'm seeing some fixes regarding this on reanimated repo can we upgrade reanimated to latest minor and check?

software-mansion/react-native-reanimated#3559

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgraded reanimated and it works!

I don't know why that issue is still open, I thought it wasn't fixed in master.

Comment on lines 4 to 9
.c0 {
display: -webkit-box;
display: -webkit-flex;
display: -ms-flexbox;
display: flex;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that all the generated CSS got removed from snapshots this should not happen as we deliberately enabled snapshots to include the CSS rules. Maybe check styled-components repo for jest 29 guides

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, it seems jest-styled-components is not compatible with jest v28+.

@@ -11,6 +11,8 @@ import type { AccessibilityMap, AccessibilityProps } from './types';
export const makeAccessible = (props: Partial<AccessibilityProps>): Record<string, unknown> => {
const newProps: Record<string, any> = {};

newProps.accessible = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might cause issues on react-native and change accessibility behaviours, because now anywhere we have used makeAccessible will cause that element to become a single selectable element and won't be able to select the nested elements.

https://reactnative.dev/docs/accessibility#accessible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anuraghazra But we are adding accessible={true} to the elements only which have other accessibility props, like accessibilityLabel, etc so it should be fine. Having other accessible props means, we do want to make the component accessible, right?

Now, it is mandatory to have accessible={true}, otherwise *ByRole queries won't return these elements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thik hai i'll test the a11y on my device once and let you know if this is causing some issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay tested each component manually on RN, most of them looks good.

There are some minor changes like in Indicator component now consumers won't be able to go into the text node and read out the "Success" text

image

But i think this is fine because it still announces the accessibilityLabel as "Status ok".
Anyway if we want to manually disable accessible prop we can just pass accessible: false when using makeAccessible, so we will just need to be a bit careful.

@kamleshchandnani
Copy link
Collaborator

@snitin315 rebase with master first so chromatic optimisations can kick in?

@snitin315 snitin315 closed this Aug 2, 2023
@snitin315 snitin315 reopened this Aug 2, 2023
@bundlemon
Copy link

bundlemon bot commented Aug 7, 2023

BundleMon

Files updated (1)
Status Path Size Limits
React Native Components
build/components/index.native.js
167.12KB (+12B +0.01%) -
Unchanged files (13)
Status Path Size Limits
Web Components
build/components/index.production.web.js
230.48KB -
React Native Tokens
build/tokens/index.native.js
28.48KB -
Web Tokens
build/tokens/index.production.web.js
27.81KB -
CSS Theme Tokens
build/css/paymentThemeDarkDesktop.css
3.7KB -
CSS Theme Tokens
build/css/bankingThemeLightDesktop.css
3.7KB -
CSS Theme Tokens
build/css/paymentThemeDarkMobile.css
3.69KB -
CSS Theme Tokens
build/css/bankingThemeLightMobile.css
3.69KB -
CSS Theme Tokens
build/css/paymentThemeLightDesktop.css
3.69KB -
CSS Theme Tokens
build/css/paymentThemeLightMobile.css
3.68KB -
CSS Theme Tokens
build/css/bankingThemeDarkDesktop.css
3.67KB -
CSS Theme Tokens
build/css/bankingThemeDarkMobile.css
3.66KB -
Web Utils
build/utils/index.production.web.js
2.92KB -
React Native Utils
build/utils/index.native.js
2.08KB -

Total files change +8B 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@@ -18,8 +18,14 @@ runs:
key: ${{ runner.os }}-blade-${{ hashFiles('**/yarn.lock') }}

# cache miss - install packages with `yarn --frozen-lockfile`
# cache hit - download packages from the cache
# cache hit - download packages from the cache and explicitly run postinstall script
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this something newly changed or github is just showing the old diff?

const files = entries.filter((file) => !file.isDirectory()).map((file) => path.join(dirPath, file.name));
const folders = entries.filter((folder) => folder.isDirectory());
for (const folder of folders)
+ if (folder.name !== 'node_modules')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats with bundlemon?

Copy link
Member

@anuraghazra anuraghazra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -0,0 +1,12 @@
diff --git a/node_modules/bundlemon/lib/main/analyzer/pathUtils.js b/node_modules/bundlemon/lib/main/analyzer/pathUtils.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise an issue on bundlemon?

@snitin315 snitin315 merged commit 6e016e1 into master Aug 7, 2023
13 checks passed
@snitin315 snitin315 deleted the feat/upgrade-jest branch August 7, 2023 09:15
anuraghazra pushed a commit that referenced this pull request Apr 9, 2024
* chore: upgrde jest to v29

* fix: create react-native-reanimated patch

* fix: tests

* chore: fix react-native issues

* test: fix react native tests

* chore: fix react native tests

* test: fix ProgressBar test cases

* test: update snapshots

* test: fix styles in snapshots

* ci: enable jest sharding

* test: update Tooltip snapshot

* test: update Tooltip snapshot

* Update .github/workflows/blade-validate.yml

* chore: update snaps

* chore: patch bundlemon

* ci: apply patches when cache-hit

* ci: apply patches when cache-hit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review - L1 First level of review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants