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

feat: Add TurboModuleRegistry.getLazy<T>(..) #43675

Closed
wants to merge 4 commits into from

Conversation

mrousavy
Copy link
Contributor

@mrousavy mrousavy commented Mar 27, 2024

Summary:

Adds TurboModuleRegistry.getLazy to get a module lazily:

const NativeMmkv = TurboModuleRegistry.getLazy<MmkvSpec>('MmkvCxx')
// ^ MmkvCxx will not yet be loaded here.

NativeMmkv.getString('test')
// ^ now MMKV will be loaded, and it will throw an error if the module is not found

Instead of requiring the module as soon as JS parses the file, it will only require the module once you try to access a property or function on the object (via a Proxy).

The idea is to increase performance and potentially also memory usage by avoiding eagerly loading modules if they are not needed immediately (e.g. only in a render function later on)

Changelog:

[GENERAL] [ADDED] Add TurboModuleRegistry.getLazy<T>(...) to lazy-load TurboModules

Test Plan:

I think we want to add this to the unit tests, right? In addition to get and getEnforcing. Not sure how those tests are set up, how do I run them?

The way I tested this was within my react-native-mmkv project by just editing RN in node_modules, and that worked :)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 27, 2024
@mrousavy mrousavy marked this pull request as ready for review April 30, 2024 13:57
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 30, 2024
Comment on lines +91 to +106

interface ModuleHolder<T> {
module: T | null
}

export function getLazy<T: TurboModule>(name: string): T {
const proxy = new Proxy<ModuleHolder<T>>({ module: null }, {
get: (target, property) => {
if (target.module == null) {
target.module = getEnforcing(name);
}
return target.module[property];
},
});
return proxy as T;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@mrousavy, in general, we want to discourage people from using Proxy. It's extremely slow: I've seen ~20x slowdown quoted internally (for Hermes). And, it may preclude some compile-time optimizations we might want to ship to react native the framework.

So, unfortunately, we cannot ship this addition to the core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, makes sense. I didn't know about these slowdowns, I'll keep that in mind when working on react-native-mmkv - thanks!

@RSNara RSNara closed this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants