feat: add `globalPrefix` to remaining require polyfill methods

[jbroma]

Summary

Part of #1480

This PR adds __METRO_GLOBAL_PREFIX__ to other require polyfill methods. With Module Federation we use multiple module system in runtime which creates a need for separating module systems from each other with globalPrefix.

Changelog: [Feature] Add global prefix to other module functions in require polyfill

Test plan

TBD

[jbroma]

Fix test to use __customPrefix__r

on the other hand, we discussed that it might not be needed to add the prefix to __r but after closer inspection it turns out it's necessary - I'm wondering whether we should introduce a separate prefix for __r or keep it as it is in this PR (prefixed with __METRO_GLOBAL_PREFIX__)?

[jbroma]

Fix test to use __customPrefix__r

on the other hand, we discussed that it might not be needed to add the prefix to __r but after closer inspection it turns out it's necessary - I'm wondering whether we should introduce a separate prefix for __r or keep it as it is in this PR (prefixed with __METRO_GLOBAL_PREFIX__)?

cc @robhogan do you think we should create a separate prefix for __r?

We've added tests to the PR and marked it as ready to review, let me know if there is anything left to be done here to proceed with this PR 🙏

[robhogan]

We'll need to update this as well so users with a custom global prefix but default getRunModuleStatement don't get broken:

https://github.com/facebook/metro/blob/35f2e48d0e2ee8161e84ca53845bd657e3745a73/packages/metro-config/src/defaults/index.js#L63-L64

[robhogan]

LGTM in principle, importing to run internal tests

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this in D77659025.

[jbroma]

We'll need to update this as well so users with a custom global prefix but default getRunModuleStatement don't get broken:

https://github.com/facebook/metro/blob/35f2e48d0e2ee8161e84ca53845bd657e3745a73/packages/metro-config/src/defaults/index.js#L63-L64

there is 2 ways to achieve this:

• we can modify the returned statement to do some runtime interpolation and include the __METRO_GLOBAL_PREFIX__ from current scope
• we can augument the getRunModuleStatement with another param that will insert the prefix at build time

what would you suggest here?

[jbroma]

@robhogan any suggestions on how to move forward with this PR?

[robhogan]

Ideally I think we add the prefix at build time - should be possible to add on top of https://github.com/facebook/metro/pull/1566

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this in D77659025.

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this in D77659025.

[jbroma]

I believe that because of changes in #1566 we should also change the default to include the __METRO_GLOBAL_PREFIX__, in a separate PR though

[robhogan]

Yes, but tbh I think these need to be the same PR or at least merged simultaneously - users may have already configured globalPrefix, but if so with the default getRunModuleStatement they'd be broken by this PR. The two together is non-breaking for a given config that customises globalPrefix. (Unavoidably breaking for users customising getRunModuleStatement)

(Related, but separately, does anyone need to customise getRunModuleStatement? I wouldn't remove it immediately but I'm inclined to deprecate it)

CC @tjzel - IIRC worklets rely on the details here.

[jbroma]

Yes, but tbh I think these need to be the same PR or at least merged simultaneously - users may have already configured globalPrefix, but if so with the default getRunModuleStatement they'd be broken by this PR. The two together is non-breaking for a given config that customises globalPrefix. (Unavoidably breaking for users customising getRunModuleStatement)

yeah, let's aim to merge those 2 one after another 👍

[tjzel]

Thanks for the ping.

We don't use getRunModuleStatement in Worklets now. We probably could to make some code less hacky but that would be only a minor improvement.

Just to make sure, after these changes, to obtain the __r function from JSI in C++ we have to make the following amendment?

- auto require = rt.global().getPropertyAsFunction(rt, "__r");
+ auto metroGlobalPrefix = rt.global().getProperty(rt, "__METRO_GLOBAL_PREFIX__");
+ auto requireName = metroGlobalPrefix + "__r";
+ auto require = rt.global().getPropertyAsFunction(rt, requireName);

[jbroma]

Just to make sure, after these changes, to obtain the __r function from JSI in C++ we have to make the following amendment?

- auto require = rt.global().getPropertyAsFunction(rt, "__r");
+ auto metroGlobalPrefix = rt.global().getProperty(rt, "__METRO_GLOBAL_PREFIX__");
+ auto requireName = metroGlobalPrefix + "__r";
+ auto require = rt.global().getPropertyAsFunction(rt, requireName);

Correct 👍

I had a quick look but it seems to be one of the only uses of that in OSS 😅 Very nice this was spotted so soon!