Page MenuHomePhabricator

[native] Enable Flow types in Reanimated worklets
ClosedPublic

Authored by angelika on Wed, Dec 11, 8:14 AM.
Tags
None
Referenced Files
F3492640: D14113.diff
Thu, Dec 19, 12:57 AM
Unknown Object (File)
Tue, Dec 17, 9:41 PM
Unknown Object (File)
Tue, Dec 17, 8:59 PM
Unknown Object (File)
Fri, Dec 13, 9:54 PM
Unknown Object (File)
Fri, Dec 13, 9:54 PM
Subscribers
None

Details

Summary

When using Flow types in reanimated worklets it looks like the types are not stripped.
https://linear.app/comm/issue/ENG-623/investigate-why-reanimated-23-is-failing-to-strip-flow-types

We’re using @babel/preset-flow which stripes the flow types. However, presets are run after plugins (https://babeljs.io/docs/plugins#plugin-ordering). That means reanimated plugin is getting the code with types.
In reanimated plugin there is a code that strips typescript types: https://github.com/software-mansion/react-native-reanimated/blob/2.17.0/plugin.js#L361-L374 so it works with ts. However it doesn’t strip Flow types. So if I add @babel/preset-flow to the code I linked it fixes the issue.
To make it nicer I made it configurable so if anyone wants to add some other plugins or presets they can do so in reanimated plugin’s options. So now after the patch we can add @babel/preset-flow in reanimated plugin options.
I’m going to make a PR with this change to Reanimated’s repo later - it’s a simple change. Made a linear issue: https://linear.app/comm/issue/ENG-10007/make-a-pr-to-reanimated-repository-to-fix-flow-types-in-worklets

Also a note to teammebers: after merging this PR they’ll need to reinstall node modules and clear metro cache. Otherwise the app will crash.

Depends on D14112

Test Plan

Verify the app doesn't crash after adding flow types in a worklet.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

angelika held this revision as a draft.

Great work on this!!

patches/react-native-reanimated+2.12.0.patch
34–42

Looks like we can delete lines 34 through 42 in this patchfile without consequence... it's just accidental linebreaks

This revision is now accepted and ready to land.Wed, Dec 11, 4:57 PM

One small thing – before landing, can you:

  1. git grep prettier-ignore
  2. Find all cases that correspond to commented-out Flow types. These work with Flow, but avoid breaking Reanimated.
  3. Remove the prettier-ignore, and replace the commented-out Flow types with normal Flow types.
  4. Either amend this diff or submit a new one if preferred.

Rebase and address feedback

native/chat/swipeable-message.react.js
21 ↗(On Diff #46410)

This line should be removed as well