Skip to content

Conversation

@kraenhansen
Copy link
Contributor

Summary:

It's been 1½ years since the merge of facebook/metro#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.

@facebook-github-bot facebook-github-bot added 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. labels Nov 11, 2023
@kraenhansen
Copy link
Contributor Author

kraenhansen commented Nov 11, 2023

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.

It's been 1½ years since the merge of facebook/metro#822 and my hope is that this feature is ready to be documented and made available through types.
@kraenhansen kraenhansen force-pushed the require-context-types branch from 3355ebc to 3519dc9 Compare November 11, 2023 20:21
@javache javache requested a review from motiz88 November 13, 2023 09:35
@motiz88
Copy link
Contributor

motiz88 commented Nov 13, 2023

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

@kraenhansen
Copy link
Contributor Author

kraenhansen commented Nov 13, 2023

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

@motiz88
Copy link
Contributor

motiz88 commented Nov 13, 2023

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

@kraenhansen
Copy link
Contributor Author

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

Why wouldn't this work?

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

@kraenhansen
Copy link
Contributor Author

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:

This is my experience in VSCode:

Screenshot 2023-11-28 at 18 48 36

@kraenhansen
Copy link
Contributor Author

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
Copy link
Contributor Author

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 kraenhansen marked this pull request as draft November 28, 2023 20:04
@motiz88
Copy link
Contributor

motiz88 commented Nov 28, 2023

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

@karlhorky
Copy link

karlhorky commented Mar 8, 2024

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:

@react-native-bot
Copy link
Collaborator

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 react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Sep 5, 2024
@karlhorky
Copy link

I'm guessing this is not stale

@react-native-bot
Copy link
Collaborator

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 react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 13, 2025
@karlhorky
Copy link

Not stale

@react-native-bot react-native-bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 14, 2025
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.

5 participants