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

Use TurboModuleRegistry.getEnforcing instead of get #6878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented Jan 8, 2025

Summary

This PR migrates all usages of TurboModuleRegistry.get to TurboModuleRegistry.getEnforcing which throws an error if the module is not found.

Test plan

@tomekzaw tomekzaw requested a review from piaskowyk January 8, 2025 16:10
@tomekzaw
Copy link
Member Author

tomekzaw commented Jan 9, 2025

cc @tjzel Perhaps we need to add a mock for WorkletsModule?

@tjzel
Copy link
Collaborator

tjzel commented Jan 9, 2025

@tomekzaw Sure, I'll add it to todos.

@tjzel
Copy link
Collaborator

tjzel commented Jan 9, 2025

Oh, I think you actually meant the breaking CI. This is not worklets fault - native modules aren't loaded in the Jest environment (same error goes for Reanimated's Turbo Module) while on the other hand Jest naively resolves all imported files, even if they aren't actually used. I looked briefly for a nice solution and got that you need to add the following to jest.config.js:

  moduleNameMapper: {
    reanimatedModuleInstance:
      '<rootDir>/src/ReanimatedModule/reanimatedModuleInstance.web.ts',
    workletsModuleInstance:
      '<rootDir>/src/worklets/WorkletsModule/workletsModuleInstance.web.ts',
    JSPropsUpdater:
      '<rootDir>/src/createAnimatedComponent/JSPropsUpdater.web.ts',
  },

However, I'm afraid that our users would also need to apply a derivative of this fix, which is kinda bad.

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.

2 participants