[No QA] Fix import of setUpTests from Reanimated

[tjzel]

Details

What

Currently Expensify uses a legacy way of acquiring setUpTests() for Jest from Reanimated which is no longer valid. Using it the old way causes type leakage from Reanimated to TypeScript in Expensify even though skipLibCheck is on and breaks type checking.

Correct way of doing it is described in Reanimated docs.

Why

Keeping this import unchanged and upgrading Reanimated causes TypeScript typechecking to fail. It's due to a leakage of types from Reanimated via the import from react-native-reanimated/src/reanimated2/jestUtils.ts, which obviously is a TypeScript source file. I'm not exactly sure how the leak happen there. The correct import should be just react-native-reanimated. However, this change causes the Jest test suite to break.

Jest

After a long struggle understanding what's happening and why, here's the summary of it:

The test suite is breaking because ESModule imports from react-native are improperly resolved in Reanimated. The file that fails is lib/module/createAnimatedComponent/createAnimatedComponent.tsx, as it's the entry point for animated components. All the ESModule imports from react-native turn out to be undefined there. However, commonJS imports via require yield correct results.

To investigate further, I simplified the test to only render a single animated component and reduced jest.config.js to only invoke setUpTests. Issue persisted.

Eventually I narrowed down the culprit of this behavior to your mocks, namely __mocks__/react-native.js. After I removed the mock, ESModule imports were working properly once again.

I dived into the react-native.js mock to understand what's causing the issue.

I'm not a Jest expert, but Jest documentation suggests to use jest.requireActual - you use Object.setPrototypeOf with an ES imported module instead. However, it didn't seem to cause any trouble. Also I'm not sure why there's no module.exports in that file.

For comparison, I created a bare React Native App, where I:

1. Imported setUpTests() there from react-native-reanimated.
2. Created a simple test there rendering a single animated component.
3. Ran the test, it passed.
4. Mocked react-native inside the test file, the way it's done in Jest documentation. Test passed.
5. Mocked react-native inside the test file as in your repo. Test passed.
6. Mocked react-native inside __mocks__. Test passed.

Then in Expensify I:

1. Copied the same test suite.
2. Replaced the contents of jest.config.js.
3. Replaced the contents of package.json, removed .lock file and node_modules, then reinstalled dependencies via npm.
4. Ran the test. It failed.
5. Replaced the mock. Test still failed.
6. Moved the mock from the file to the test. It worked, but I noticed that the mock is not applied at all in this configuration. It's not the case in the bare App repo.

It seems that something is wrong with how/when the mocks are loaded, but I couldn't find the culprit of that and decided not to investigate further.

I came up with two solutions to this problem:

1. Fix troublesome imports in Reanimated with require via patch-package in troublesome places.
2. Since in unit tests in Expensify you don't actually use the tools Reanimated provides in setUpTests (it loads new matchers to check styles of animated components), you can just drop it.

The latter here seems obviously superior to me - the patch for now would be just a few line in a single file, but it's definitely not a scalable solution and requires maintenance, had something changed in Reanimated repo. Therefore removing that function call sounds like a smart move.

I'd suggest investigating what's wrong with the mocks evaluation, had you wanted to extend your Jest tests significantly.

Results

npm run typecheck command will work properly now, it fails on CI (and locally) in #36404.

npm run test passes on Reanimated 3.6.1 (current) and on 3.7.0.

Side note

I'm not sure that tsconfig.json is flawless. When I was debugging this problem I was fiddling with the include and exclude section there, and even though I'd put for e.g. src from include to exclude it was still type checked (and with errors). However, I don't know the structure of your repo that well, so I might be mistaken. On the other hand, I know that tsconfig.json is a pain to set-up correctly and that probably nobody in the world can do it 100% right, so I'm not judging.

Fixed Issues

$ #36187

Tests

1. npm run test - pass.
2. npm run typecheck - pass.
3. npm install react-native-reanimated@3.7.0
4. npm run test - pass.
5. npm run typecheck - pass.
   🥳

• Verify that no errors appear in the JS console

Offline tests

Same as tests.

QA Steps

Irrelevant AFAIK.

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Irrelevant.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[tjzel]

I have read the CLA Document and I hereby sign the CLA

[melvin-bot]

@amyevans Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[amyevans]

Just run npm run typecheck now with Reanimated 3.7.0. and see that it works now 🥳.

3.7.0 isn't the current version in main, is this PR intended to be opened against main?

Also is there a GH associated with this change?

[tjzel]

Hi @amyevans. I was simply asked to help with bumping Reanimated by @BartoszGrajdek since it was a blocker for him. I believe this should be opened against main since it fixes the issue of misusing that import. However, after diving into what goes on in the CI and why Jest tests are breaking I see that the problem is a bit more complicated (and I don't understand what is exactly happening). I will move it back into a draft to try and figure it out.

Also, what is GH?

[amyevans]

Okay all good, thanks for digging into it further. And sorry, GH = GitHub. Typically each PR opened against this repo has an associated GitHub issue with further context of the problem being fixed and assigned team members/reviewers.

[amyevans]

Thanks for adding the extra detail. LGTM, but I also wouldn't mind if @roryabraham put 👀 on this.

[amyevans]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

[roryabraham]

Tests are passing, so LGTM 👍

[OSBotify]

🚀 Deployed to staging by https://github.com/roryabraham in version: 1.4.43-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.43-20 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅