-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: define "react-native" community condition #45367
doc: define "react-native" community condition #45367
Conversation
Review requested:
|
There’s a spec in progress for this: nodejs/tooling#166. I assume we should defer to that spec? cc @nodejs/tsc @guybedford |
+1, but this submission aligns with it so far afaict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generically +1, with the WinterCG caveat
@GeoffreyBooth Thanks for raising, we also gave input on the WinterCG spec. Worth noting that
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi all, is this good to merge? :) |
Landed in d09f0c4 |
PR-URL: nodejs#45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #45367 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I'm a React Native maintainer working on Metro, our first-party JavaScript bundler. Here I'm proposing the inclusion of a
"react-native"
condition under the "Community Conditions Definitions" section of Node's docs.This is linked to our framework proposal for package exports support: react-native-community/discussions-and-proposals#534.
Why this condition is needed
React Native is a popular platform within the JS ecosystem, where code can be executed in non-browser environments, such as our Hermes JavaScript engine. When package maintainers want to target React Native, we have historically provided a top-level
"react-native"
root field withinpackage.json
— which was an extension of the"browser"
field spec (now itself a community condition).e.g. Used by pusher-js:
As with
"browser"
, we are aiming to translate"react-native"
to the conditional exports concept.What the
"react-native"
condition will representThese are unchanged from current use of the
"react-native"
root field.What will change when this is used as a condition via
"exports"
:"exports"
allows packages to define the order of conditional exports, package maintainers will have more fine-grained control over what is selected for their package (e.g. matching before, or after,"browser"
).Prior usage
Use of
"react-native"
root fieldThis is a popular feature and is used by both React Native-specific libraries and larger multiplatform libraries (such as the above pusher-js example). Searching across GitHub, use of the top-level
"react-native"
field for specifying alternate source files is present in ~20k repositories.Use of
"react-native"
package exports conditionWebpack already lists
"react-native"
under its docs for Package exports, which is enabled when configured by the user or in templates under React Native projects.From this we can infer there is independent desire for such a condition, and perhaps limited use today. The definition it provides is "TBD", leaving us to provide the concrete and canonical definition with this PR.
How we teach this
Once "exports" support is ready in Metro:
With the merging of this PR: