react-native icon indicating copy to clipboard operation
react-native copied to clipboard

Update types for `require` and `require.context`

Open kraenhansen opened this issue 2 years ago • 12 comments

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.

kraenhansen avatar Nov 11 '23 20:11 kraenhansen

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.

kraenhansen avatar Nov 11 '23 20:11 kraenhansen

I think we need to ship the feature as stable (and on by default) before we ship types that assume it's on.

motiz88 avatar Nov 13 '23 11:11 motiz88

@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.

kraenhansen avatar Nov 13 '23 18:11 kraenhansen

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).

motiz88 avatar Nov 13 '23 19:11 motiz88

We don't support feature-detecting require.context in this way.

Why wouldn't this work?

if (require.context) {
  require.context(/* ... */);
}

kraenhansen avatar Nov 13 '23 23:11 kraenhansen

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:

Screenshot 2023-11-28 at 18 48 36

kraenhansen avatar Nov 28 '23 17:11 kraenhansen

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.

kraenhansen avatar Nov 28 '23 19:11 kraenhansen

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.context is not enabled in your project. This can be enabled by setting the transformer.unstable_allowRequireContext property to true in 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.

kraenhansen avatar Nov 28 '23 20:11 kraenhansen

To be clear, at the point when we enable require.context by default (or soon afterwards), we'll probably remove the option entirely.

motiz88 avatar Nov 28 '23 20:11 motiz88

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

karlhorky avatar Mar 08 '24 12:03 karlhorky

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.

react-native-bot avatar Sep 05 '24 05:09 react-native-bot

I'm guessing this is not stale

karlhorky avatar Sep 05 '24 07:09 karlhorky

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.

react-native-bot avatar Aug 13 '25 05:08 react-native-bot

Not stale

karlhorky avatar Aug 13 '25 05:08 karlhorky