-
Notifications
You must be signed in to change notification settings - Fork 128
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: introduce reactNativeManifest
to package.json
#588
Conversation
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.
Thanks for the writeup @kelset
I'll do a further review in the next days. I'll just left one thoughts I had on top of my mind
|
||
## Alternatives | ||
|
||
`reactNativeMeta` is, by design, fully original and new: there are other files that are used to define configurations for various aspect of react-native based projects (such as `react-native.config.js`, `app.json`, Expo's `app.config.js`, `expo-module.config.json`) but this one does **not** overlap with any of them. Explicitly, this one has the purpose of being filled only with metadata to be read by package managers and other tools. |
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.
As this grows, or if we want to allow it to be dynamic, or extendable from a base, it seems beneficial to allow mirroring into a react-native.config.js
outside the direct package.json (maybe not named that since already taken). E.g. see recent template change moving Jest config out of package.json (or... cosmic-config bits).
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.
react-native.config.js
already exists and is used for CLI stuff (see https://github.com/react-native-community/cli/blob/main/docs/configuration.md#configuration) - it is an explicit goal of this metadata field to be entirely separate.
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.
Call it something else as an example, but it seems like the app fields are being used for configuration. If that configuration becomes non-trivial, it may scale badly inlined into each package.json vs being shareable. Like, say, a root eslint.config.js
or jest.config.js
. The mapping of foo
in package.json to foo.config.js
is also somewhat expected by convention imo.
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.
filled only with metadata to be read by package managers and other tools
What does this mean? It seems like it allows it to be used for almost anything.
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.
As this grows, or if we want to allow it to be dynamic, or extendable from a base, it seems beneficial to allow mirroring into a react-native.config.js outside the direct package.json
Could you clarify on why we want this now? This is something we can still add later if we wish and the current design is not preventing it. Or am I missing something?
reactNativeMeta
to package.jsonreactNativeMetadata
to package.json
High-level comment: I don't think solving for apps and libraries with the same mechanism is the right call. |
hey folks - quick update here: there have been some conversations happening behind the scenes but current goal is to get to a new level of alignment on the topic and update the PR here by end of next week |
I've spent some time reading this RFC and talking to @cortinico, and I've identified my main issues with it. I'll start with the more focused points, with broader concerns towards the end. Autocomplete / IDE supportIf we add a field to NamingThis is a small & bikesheddy point, but I'm gravitating towards Scope creep (current and future)A "bag of properties" config paradigm can easily end up confusing people by conflating properties that apply at different times, to different scopes, and have different levels of strictness / user-friendliness. In short, we need to ensure this mechanism stays cohesive and easy to learn. We should apply a lot of scrutiny to potential use cases here and lay down stronger guidelines on what's a good fit for The current proposal introduces
I think that's too open-ended. I'd propose wording to the effect of:
Concretely:
What is the big picture design?tl;dr for the purposes of this RFC: we should not block on solving this much larger challenge, BUT we should ruthlessly limit the scope to what's immediately useful so that we can focus future efforts on a more comprehensive solution. To put it bluntly, it feels like we're dancing around the lack of an end-to-end, user-extensible, designed, build system for native code in React Native projects. We do have a patchwork of systems in place, that many people are using successfully, but the ad hoc nature of these solutions leaves us with a poor caching/incremental builds story, scattered documentation, duplication of tools/effort, wildly differing levels of polish, and - yes - an unclear source of truth for build configuration in a project. AFAICT, Concretely, we should put resources towards designing the overall build system for React Native, and think of our current efforts as the first few steps towards that broader vision. That cohesive system may or may not be based on Buck; it's much too early to take a firm stance either way. (And frankly this idea might not even materialise soon - for now I'm mainly hoping to start a discussion around it.) But the concept of specifying a single cross-platform graph of build actions, encompassing all the configuration, codegen, compilation, linking, and bundling in a React Native project, is something that Buck makes relatively natural - so I think it's one of the options we should evaluate more closely. To be clear, there are tough design problems here, e.g. how the unified build system should interoperate with Xcode / Android Studio / and their underlying build systems & package managers - and I'm not claiming to have complete answers at this stage. But there are encouraging precedents at Meta for tools that do glue Buck with Xcode and Android Studio while providing a good and consistent developer experience. |
Hey all, |
I want to push back against this point - the That said, I do appreciate your point about tightening this specification up and I think we could simplify the
|
As of today, there is no simple programmatic way to know if a NPM package is a React Native library. Good indicators could be: | ||
* Presence of a `react-native:*` dependency in the `peerDependencies` section of the `package.json` ([example](https://github.com/RonRadtke/react-native-blob-util/blob/80628d418a5c81439dc2c1a1f05fae6406f0ba7f/package.json#L46C10-L49)) | ||
* Presence of React Native specific files such as `react-native.config.js` files | ||
Those indicators are all non-exhaustive. |
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.
How do other libraries determine this? I guess it's not really a problem because we need to be able to catalogue libraries specifically for React Native?
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.
How do other libraries determine this?
What do you mean with "other libraries". Like other ecosystems?
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.
Oops yes other ecosystems. I'm guessing they have similar directories. Are they all maintaining it manually?
|
||
Most of those config flags are undocumented and they're not verified for invalid configuration. Specifically, some feature flags require to be toggled on multiple layers, and it's easy to forget to toggle one of them. Invalid combinations of those flags could lead to unexpected runtime behavior. | ||
|
||
We propose to define `reactNativeManifest` as the **preferred entry point** for all the user facing entry points. All the tooling should be adapted to consume the information from the `reactNativeManifest` section and have sensible defaults defined by the tooling itself. |
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.
Is this only for apps? I'm guessing it doesn't make sense for a library to specify flags? But I guess there could be a library that enforces a certain feature flag?
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.
It's for both apps and libraries. There could be a scenarios where a capability needs all the dependencies to expose such capability (for example the newArch
one).
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.
Would this proposal enforce this? Like if a library specifies a certain feature flag, and an app uses that library, how do we communicate this?
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.
Would this proposal enforce this? Like if a library specifies a certain feature flag, and an app uses that library, how do we communicate this?
That's up to each feature to define the semantic of such flags. That's also why we need a dedicated RFC for each feature. There might be features that have a hard requirement on dependencies (like the New Architecture Renderer) and other features that have fallbacks
reactNativeMetadata
to package.jsonreactNativeManifest
to package.json
Thanks for flagging this @brentvatne. I think we'll probably have a separate RFC to decide how to proceed with either I do personally like your direction of allowing only |
The |
For context, that's how it's called today: |
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.
I left some comments. Most of them are fixes for typos, but I left also a couple of questions.
|
||
Therefore we believe `align-deps` can benefit from this information and be extended to offer New Architecture support information. | ||
|
||
### React Native CLI support |
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.
would this be a part of React Native core or a feature we want to defer to frameworks?
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.
This is framework/tooling responsibility. Is added here to provide context on what we believe could be valuable use cases of those APIs
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.
I left some comments. Most of them are fixes for typos, but I left also a couple of questions.
Hey folks, I created a(nother) PR with some small updates on this. |
This RFC wants to introduce and formally discuss a new section for the package.json of a react-native project (app or library), called
reactNativeMetadata
.This new section will allow developer to express certain characteristics of their code in a more formalized manner, that can easily used by tooling to act towards the code accordingly.
Note well: this is but the first draft of the actual shape and fields. The goal of this draft is to collect feedback and help finalise the spec for this section.
You can read more details in the text itself - here's the file view for easier reading experience.