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

fix: Fix react-native-reanimated is not installed! error #2953

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

mgefimov
Copy link
Contributor

@mgefimov mgefimov commented Jun 9, 2024

What

This PR fixes the errors react-native-reanimated is not installed!, @shopify/react-native-skia is not installed!, and react-native-worklets-core is not installed! that occur when inlineRequires is disabled in metro.config.js (enabled by default).

Capture333333

When inlineRequires is enabled, there are no errors because all modules are loaded lazily.

When inlineRequires is disabled, there is an attempt to read the property $$typeof at start time from a module created via createModuleProxy, which causes an error.

Changes

This PR changes createModuleProxy to return an object that contains the proxy instead of returning the proxy directly. This prevents unnecessary access to the proxy.

We could, for instance, add a check only for the $$typeof property, but I think the proxy doesn't need to be aware of this property. Additionally, I'm not sure if it will work if we create a proxy for a react element module.

Tested on

  • iPhone 13, ios 15.5
  • Nexus 4 Emulator, Android 13

Related issues

Minimal reproducible example: https://github.com/mgefimov/vision-camera-inline-requires-error

  1. Init new RN app, react-native version 0.71.0
  2. Install react-native-vision-camera, setup camera code
  3. Disable inlineRequires
    Screenshot_1717927341

Copy link

vercel bot commented Jun 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2024 2:59pm

@mgefimov mgefimov changed the title fix: Fix metro inline requires disabled error fix: Fix react-native-reanimated is not installed! error Jun 9, 2024
@mrousavy
Copy link
Owner

hey! thank you for the PR, good stuff!

I think it looks kinda weird with the .module prefix everytime though... I think I'd prefer overriding $$typeof in the Proxy, do you see any downsides with that approach?

@mgefimov
Copy link
Contributor Author

There might be a problem if we create a proxy for React element module. If we want to support React element module, the solution might be a bit more complicated.

@mgefimov
Copy link
Contributor Author

mgefimov commented Jun 10, 2024

For almost all cases, we can return undefined, but for a module like this:

import React from 'react'
import { Text } from 'react-native'

const Hello = () => <Text>Hello, world</Text>

module.exports = React.memo(Hello)

we should return Symbol(react.memo). However, if the module is not installed, we should return undefined. This might complicate the solution; what do you think?

@mrousavy
Copy link
Owner

Hm I don't think we'll ever have such modules, but I'm not sure if that's the case.

Do you prefer .module, or overwriting $$typeof?

@mgefimov
Copy link
Contributor Author

In that case, I think overwriting $$typeof is a better option. I have updated the PR

@mrousavy
Copy link
Owner

This is really a weird fix, I hope this is the right solution forward. I don't know what side-effects will be introduced if we override $$typeof and return undefined even though it is an object/proxy.

Either way, LGTM - thank you so much for your contribution and explanation on this!!

@mrousavy mrousavy merged commit 573ab81 into mrousavy:main Jun 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants