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: use @react-native/assets-registry #2682

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EvanBacon
Copy link
Contributor

  • Metro has a single string value that it uses for registering assets, and this value isn't multi-platform.
  • When an asset is found by Metro, this value is used to register the module ID for runtime lookup in modules such as Image.
  • To support both native and web platforms in Expo CLI, we alias the resolution for react-native-webs asset registry to a shared module ID to ensure getAssetByID can find the same module that Metro uses for registerAsset here.
  • This PR simplifies the logic by using the same module as React Native.

@necolas
Copy link
Owner

necolas commented Jun 10, 2024

Looks like the unit tests and builds fail because the new dependency contains uncompiled JS code.

FAIL packages/react-native-web/src/exports/Image/__tests__/index-test.js
  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

There's no main export from the package either - see code published to npm.

Compare that to normalize-colors, which looks like it has compiled code with the Flow types removed, and has an index.js file (which I guess is assumed as the entry if pacakge.json is missing that field).

I think the assets-registry package needs upstream fixes to at least match what normalize-colors does.

Copy link
Owner

@necolas necolas left a comment

Choose a reason for hiding this comment

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

Upstream package needs fixes

@headfire94
Copy link

flow removal PR: facebook/react-native#44002

@headfire94
Copy link

headfire94 commented Sep 24, 2024

For those who don't want to wait i solve it using:

  1. babel alias '@react-native/assets/registry': 'react-native-web/dist/cjs/modules/AssetRegistry', so that mobile request AssetRegitry from web (code is the same)
  2. assetRegistryPath: 'react-native-web/dist/cjs/modules/AssetRegistry/index.js' in metro config` so that metro use same instance

@necolas necolas added the bug: react-native Bug associated with upstream React Native vendor code label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: react-native Bug associated with upstream React Native vendor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants