Update types for `require` and `require.context`
Summary:
It's been 1½ years since the merge of https://github.com/facebook/metro/pull/822 and my hope is that this feature is ready to be documented and made available through types.
Changelog:
[GENERAL] [ADDED] - Added types for require.context, which is provided by Metro at build time.
Test Plan:
I've tested this manually in a project.
cc @EvanBacon What do you think? Is this ready for general consumption? I would love to rely on these types in examples of another package of mine.
I think we need to ship the feature as stable (and on by default) before we ship types that assume it's on.
@motiz88 great point! I honestly don't know why I didn't realize this wasn't enabled by default. How about making the context property optional, forcing code to check for it's presence before relying on it? I'll updated the PR accordingly.
Unfortunately I don't think making it nullable is sufficient. We don't support feature-detecting require.context in this way. The best way forward is to enable it out of the box, then document it (including in types).
We don't support feature-detecting require.context in this way.
Why wouldn't this work?
if (require.context) {
require.context(/* ... */);
}
Trying to verify this fix this today, I see that this change conflicts with the require global declared by @types/node which are included into the project, at least one place:
- https://github.com/facebook/metro/blob/main/packages/metro-core/types/index.d.ts#L11
This is my experience in VSCode:
Because of NodeRequire being declared in @types/node/globals.d.ts and it being automatically included in the project, this PR currently needs a declare const to pin the type of require in user-space:
declare const require: ReactNativeRequire;
if (require.context) {
require.context('.');
}
I hope to investigate other workarounds for this.
Never mind, now I noticed there's runtime code throwing if we simply get context off the require object: I suspect that's the reason we can't feature-detect it 😞 thanks for mentioning this @motiz88.
ERROR Error: The experimental Metro feature
require.contextis not enabled in your project. This can be enabled by setting thetransformer.unstable_allowRequireContextproperty totruein your Metro configuration.
Even if we enable this by default, the types won't be accurate if users disable it - perhaps that's good enough?
The TS issue of require being declared as NodeRequire remains 🤔 I'll draft this for now.
To be clear, at the point when we enable require.context by default (or soon afterwards), we'll probably remove the option entirely.
For anyone looking for a copy+paste metroRequire.d.ts for TypeScript types for Metro context.require() to add to your own project until this PR is merged, @EvanBacon has one here:
- https://github.com/EvanBacon/Metro-Context-Modules-Demo/blob/3ad84a12b40c51060ee50d0b0065d02fd8ce3b43/types/metroRequire.d.ts
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
I'm guessing this is not stale
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
Not stale