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

RFC: Metro package exports support #534

Merged
merged 5 commits into from
Nov 27, 2022

Conversation

huntie
Copy link
Collaborator

@huntie huntie commented Oct 17, 2022

This adds a new RFC containing the intended design and rollout strategy for package exports support within Metro, which will affect how React Native apps consume npm packages and how package authors expose modules to React Native apps.

NOTE: This is a mostly-ready first pass which I'm opening now in order to link related conversations to. Further commits will be stacked to address early feedback.


Recommended (in the Files changed tab): "Display the rich diff"
image

proposals/0534-metro-package-exports-support.md Outdated Show resolved Hide resolved
proposals/0534-metro-package-exports-support.md Outdated Show resolved Hide resolved
proposals/0534-metro-package-exports-support.md Outdated Show resolved Hide resolved
@huntie
Copy link
Collaborator Author

huntie commented Oct 20, 2022

Near-term planned changes

From initial internal and external feedback round. These are being strongly considered to form updated versions of this proposal.

cc @motiz88 @robhogan @EvanBacon @kelset (+ others)

1. Move strict handling out of current proposal

The surface area of this proposal is large and we want to be sure that package maintainers understand changes correctly and are not burdened by too many framework decisions at once (e.g. the new architecture migration is concurrently being pushed to the community).

Most of the benefits of "exports" relevant to React Native app developers will be adequately delivered by the proposed lenient handling by Metro, and "metroExportsMode": "strict" presents complexity and footguns both to us and to package maintainers in implementing this correctly. Therefore it makes sense not to ship metroExportsMode immediately.

Proposed: Split out all strict mode references into a future RFC (for a future effort).

2. Widen definition and behaviour of "react-native" condition

Expand the "react-native" condition to represent all React Native platforms (more conceptually and community aligned), provide info on how packages should correctly use this with "browser" for react-native-web support (i.e. guide the introduction of the "react-native" condition correctly).

Proposed: Metro will assert both conditions.
Possible alternative: Metro gives precedence to "browser" when platform = 'web' — but non-spec compliant.

Relevant discussion: #535 (comment)

3. Handling of platform-specific extensions under "exports"

The current plan of deciding to try extra files based on extensionless "exports" keys is very loose behaviour and possibly spec-conflicting enough to break in other bundlers.

This could be redesigned now or deprioritised and handled as a future decision + feature — particularly as we offer alternatives for the same functionality without platform extensions.


Note: "Near-term" = reasonably soon. I'm likely to focus on other internal project priorities before getting back to this.

@Andjeliko Andjeliko added the 💡 Proposal This label identifies a proposal label Oct 20, 2022
@motiz88
Copy link
Collaborator

motiz88 commented Oct 25, 2022

Re asserting both "react-native" and "browser" everywhere and proceeding as per spec (picking the one specified first if both are specified): I think this is the right call as the least breaking change that preserves the most future value. IMO it goes hand-in-hand with keeping the default resolverMainFields: ["react-native", "browser", "main"] setting in the CLI. The key difference is that with exports, package authors wishing to provide both browser and react-native entry points can control which one gets selected.

@kelset
Copy link
Member

kelset commented Nov 10, 2022

hey @huntie just wanted to check in - what's the status of the work and/or of this PR? any blockers we can help with?

@huntie
Copy link
Collaborator Author

huntie commented Nov 11, 2022

@kelset No blockers as of the "Near-term planned changes" note above. I haven't started implementation yet due to internal project scheduling (have been working on another thing for the last couple of months), but I wanted to get questions answered for "exports" earlier and get this RFC (and Node RFC) in public.

I'm aiming to push the updated RFC doc (incorporating planned changes) to this branch next week, then seeking to merge this PR. Will ping you and others for an approval at that time.

@SimenB
Copy link

SimenB commented Nov 11, 2022

FWIW, facebook/react-native#35203 has landed, so the Jest env used in the preset uses it (or, will when published)

@huntie
Copy link
Collaborator Author

huntie commented Nov 17, 2022

Changes in 1a25973

(Planned changes above were integrated in e510faa.) Following this, I've pushed another revision addressing further feedback, and have ended up inverting our plans around backwards compatibility for exact path specifiers and platform-specific extensions — we would prefer to make handling of these cases breaking in order to follow the "exports" spec correctly (discussed with @motiz88).

Significant changes:

  • Now breaking: "exports" subpath is preferred in a path conflict.
  • Now breaking: "exports" subpath is used if matched, blocking platform-specific extensions resolution.

More detail on:

  • Dynamic configuration of asserted condition names.
  • Anticipated breaking change risk levels.
  • Nested conditions support is now P0.

As of these changes, I'm looking to have the proposal accepted and merged.

@huntie huntie force-pushed the metro-exports-rfc branch 2 times, most recently from c897cba to 15e943a Compare November 18, 2022 12:09
@matthew-dean
Copy link

What about subpath imports? Is there a separate RFC for this?

@huntie
Copy link
Collaborator Author

huntie commented Nov 23, 2022

@matthew-dean Briefly noted on L78: We aren't committing to implementing subpath imports right now, but it might be something we can follow up with quickly next year.

Copy link
Collaborator

@motiz88 motiz88 left a comment

Choose a reason for hiding this comment

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

This is stellar work! Thanks @huntie for shepherding it through all the design discussions. LGTM, with one thing I'd love to see clarified re: path conflicts.

…g detail

Significant changes:
- Now breaking: `"exports"` subpath is preferred in a path conflict.
- Now breaking: `"exports"` subpath is used if matched, blocking platform-specific extensions resolution.

More detail on:
- Dynamic configuration of asserted condition names
- Anticipated breaking change risk levels
- Nested conditions support is now P0
@huntie huntie merged commit b78f018 into react-native-community:master Nov 27, 2022
huntie added a commit that referenced this pull request Nov 27, 2022
@huntie huntie deleted the metro-exports-rfc branch November 27, 2022 15:33
nabeelr7 added a commit to nabeelr7/discussions-and-proposals that referenced this pull request Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Proposal This label identifies a proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants