-
Notifications
You must be signed in to change notification settings - Fork 132
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
Extract CLI from core #13
Conversation
|
||
## Motivation | ||
|
||
Facebook does not use CLI provided with React Native, which makes it hard to maintain the project. There is no clear owner and issues / pull requests get stack in the sea of other, non-CLI related reports. |
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.
s/stack/stuck
Left a couple of questions but all in all this seems pretty reasonable. |
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.
👍 👍 👍
Looking forward to seeing this implemented!
|
||
## Motivation | ||
|
||
Facebook does not use CLI provided with React Native, which makes it hard to maintain the project. There is no clear owner and issues / pull requests get stack in the sea of other, non-CLI related reports. |
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.
s/use CLI/use the CLI
|
||
Having a separate package with me (and Callstack) being strategic maintainers would allow us to iterate on the design of CLI even more and make sure we have the best tooling out there. | ||
|
||
Now, if you have been following React Native development for a few years, you know I have been always super excited about CLI in general. I did many talks in that field and lead efforts of creating RNPM and then, merging it to the core as `react-native link` together with Alexey Kureev. Link, and RNPM, in general, was just the first step on our way to meet that high expectation of high-quality tools. Unfortunately, due to the aforementioned constraints and lack of time we were never able to complete our long-term strategy. |
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.
Now, if you have been following React Native development for a few years, you know I have been always super excited about CLI in general
I think it might be best to keep anecdotal things out of the context of these proposals, so that folks don't need a history lesson or feel they need to know someone in the industry to participate.
While I appreciate you're excited by this (and I know you'll be a great maintainer!), I'm not sure your personality contributes to the technical impact that this will have. 😄
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.
Sure, thanks for leaving this feedback. I guess it's due to my understanding of "motivation" that I took a bit personal as well, rather than focusing on "technical motivation" in the first place.
|
||
Apart from reorganizing the architecture, we would like to create a website that describes the features present in the CLI - many of them, making it much easier to work at a scale. Things like support for different locations of iOS/Android projects, linking assets from 3rd party modules are great features that only selected people were fortunate to learn about. | ||
|
||
As a next step, we would like to focus on reinvestigating every single piece of CLI to see what we can do better these days. That includes looking once again into the design of the scaffolding system (that is great, but has its limitations), lock-in to Metro (because why it should be the forced default?) and rewriting link to support modern languages and new module system. |
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.
rewriting link to support modern languages and new module system.
This seems like it could be its whole own proposal... 😨
What do you think about keeping the proposal limited to just the extraction of the CLI for now?
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.
Yes, I gone too deep with my plans for the CLI in general. I will discuss them today on the meeting and get some initial ideas, but for now, I am going to remove 50% of this proposal and just make it about extracting from the repository itself.
|
||
## Drawbacks | ||
|
||
There are no drawbacks, the whole migration process happens as a part of one of the future React Native versions w/o users noticing. The separate Github repository is just for better tracking of issues. |
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.
One drawback I can think of is the separation of code, and thus repositories, leading to folks not know where something lives or is implemented. While it will be good to track issues in two places, how will developers know which repo to file issues on? It may require some informing of developers to better understand the architecture.
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.
We've definitely felt this pain as a result of our CLI (https://github.com/infinitered/ignite) and boilerplate (https://github.com/infinitered/ignite-ir-boilerplate-bowser) being separated.
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 isn't a reason not to separate them, though. It's been helpful overall!)
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.
Yeah, I guess we should better communicate that react-native-cli
is a separate package. I can think of an automated message printed after every failure in CLI educating that this "error report" can be submitted to this repository in particular.
|
||
## Detailed design | ||
|
||
For the first release, the goal is to just extract out the CLI into a separate package. Since every command is a standalone module, we would like to use Yarn workspaces to build a modular architecture around it. |
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.
Can you elaborate on this? I’m not very familiar with how the cli works so this section isn’t very clear to me.
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 am not sure if it makes sense to have every command in a stand alone module - may main maintenance hard. I would also encourage taking a look at the CLIs for Cordova and NativeScript - which have a similar modular story, but not necessary in separate modules.
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.
Well, right now, commands are standalone packages anyway, it's just that we don't expose it elsewhere. I don't think it would be useful to do it anyway. I was initially thinking about an internal structure to manage dependencies. Can do it with single package.json
anyway, but was thinking Yarn workspaces are a very light-weight addition.
|
||
For the first release, the goal is to just extract out the CLI into a separate package. Since every command is a standalone module, we would like to use Yarn workspaces to build a modular architecture around it. | ||
|
||
Not many people know, but not only CLI allows you to provide your custom commands but also to override the built-in ones. That makes it even more obvious to create a package-oriented setup. The similar setup is already followed by React and Metro and we would take learnings from there. |
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.
What is an example of a command that overrides built in ones? Is this something specifically for supporting additional platforms?
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-windows. I am not sure if that was the right approach in the first place. We probably should have had something like react-native add platform react-native-windows
.
Also, what is the advantage of making the CLI pluggable, when you could technically just add node_modules
and run those !!
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 easier to set up and works out of the box - no point in worrying about some common things as error handling, params, and cross-platform capability. Also, most importantly, CLI is equipped with "config" object (referred to as "context" in the scope of every command) that gets passed to every command as a first argument. It contains an information about the source files of React Native project, available platforms etc. so that users that want to write their own plugin to perform native work don't have to guess things on their own. React Native link
command uses that object to do all its work.
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 part needs some love and definitely to be discussed. The pluggable architecture is an internal implementation detail, but I've found it very easy and cheap to open to the public.
|
||
The biggest part of the `local-cli` is its `core`, which is capable of analyzing your source code. It provides a context object for all the commands, that contains necessary information about the location of your source files, available platforms and more. Commands such as `link`, `unlink` or `run-ios` use that information to perform their work. | ||
|
||
This proposal suggests no changes to the end user in its first iteration. We would simply couple new package with React Native version, just like we do with Metro. That makes it possible to still make React Native version specific changes to the CLI 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.
I feel like we’ve often run into problems with metro versions being so tightly coupled to RN releases. Will that not be a problem here?
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.
Not really - we will still bundle the CLI with React Native to start with (just like we bundle Metro as a dependency). In reality, there are less and less features that need to be updated for new React Native releases. In most cases, it's due to Metro changing the interface, which we can track here as well.
|
||
Extract the current CLI implementation from React Native Core and move it into a third party package. | ||
|
||
## Basic example |
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.
Should we just call this section "Background", even in the template ? Looks like almost all proposals seem to need a background section, talking about the state of the world.
A few questions
|
|
||
The biggest part of the `local-cli` is its `core`, which is capable of analyzing your source code. It provides a context object for all the commands, that contains necessary information about the location of your source files, available platforms and more. Commands such as `link`, `unlink` or `run-ios` use that information to perform their work. | ||
|
||
This proposal suggests no changes to the end user in its first iteration. We would simply couple new package with React Native version, just like we do with Metro. That makes it possible to still make React Native version specific changes to the CLI 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.
I think we need to define what the features of "first iteration" really are, before declaring that there would be no changes. Maybe more this sentence to later ?
|
||
It is also hard to keep moving on the CLI as every additional dependency or code change is subject to the heavy review process and sync. | ||
|
||
Having a separate package with me (and Callstack) being strategic maintainers would allow us to iterate on the design of CLI even more and make sure we have the best tooling out there. |
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 sounds more like a pitch to take over the CLI. Also, if this happens, why would this be any different from any of the other CLIs for RN that exist out there. How would this help ?
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 not - sorry if it reads that way. The whole point of this paragraph was to emphasize the fact that having CLI as a separate package will make it easier to consolidate work in a more focused repository without all the Facebook sync infrastructure.
|
||
## Detailed design | ||
|
||
For the first release, the goal is to just extract out the CLI into a separate package. Since every command is a standalone module, we would like to use Yarn workspaces to build a modular architecture around it. |
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 am not sure if it makes sense to have every command in a stand alone module - may main maintenance hard. I would also encourage taking a look at the CLIs for Cordova and NativeScript - which have a similar modular story, but not necessary in separate modules.
|
||
For the first release, the goal is to just extract out the CLI into a separate package. Since every command is a standalone module, we would like to use Yarn workspaces to build a modular architecture around it. | ||
|
||
Not many people know, but not only CLI allows you to provide your custom commands but also to override the built-in ones. That makes it even more obvious to create a package-oriented setup. The similar setup is already followed by React and Metro and we would take learnings from there. |
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-windows. I am not sure if that was the right approach in the first place. We probably should have had something like react-native add platform react-native-windows
.
Also, what is the advantage of making the CLI pluggable, when you could technically just add node_modules
and run those !!
|
||
Not many people know, but not only CLI allows you to provide your custom commands but also to override the built-in ones. That makes it even more obvious to create a package-oriented setup. The similar setup is already followed by React and Metro and we would take learnings from there. | ||
|
||
Apart from reorganizing the architecture, we would like to create a website that describes the features present in the CLI - many of them, making it much easier to work at a scale. Things like support for different locations of iOS/Android projects, linking assets from 3rd party modules are great features that only selected people were fortunate to learn about. |
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 should be done, regardless of this current effort. Not sure if it makes sense to keep it here.
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 certainly one of the features on the roadmap to take right away once the cut is done. I didn't have a better place to track that. Will remove it for now and keep it under a separate issue.
|
||
As a next step, we would like to focus on reinvestigating every single piece of CLI to see what we can do better these days. That includes looking once again into the design of the scaffolding system (that is great, but has its limitations), lock-in to Metro (because why it should be the forced default?) and rewriting link to support modern languages and new module system. | ||
|
||
The list of features is much longer and as a someone who believes tools are to give framework success on the market, I want to make sure we get the best tooling possible. |
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 think this is also an abstract statement. Does it make sense to define what exactly we do here ?
|
||
There are no drawbacks, the whole migration process happens as a part of one of the future React Native versions w/o users noticing. The separate Github repository is just for better tracking of issues. | ||
|
||
In the future, we might consider decoupling these two, but at the time of writing this document, I do not have answers to many questions arising from such a move. |
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 also should be separate, no point adding it here.
Thanks, everyone for leaving comments on this proposal. This was the first one I have ever written - appreciate the comments, will get back to every single one when I am back from holidays next Monday. |
@axe-fb for some reason, I couldn't answer all your comments, due to Github UI not showing the "comment" box. I am in the process of refactoring this proposal right now. Please see the diff in a few hours for the updated version and let me know what you think. Once again, thanks for leaving comments on this proposal and helping me get it up and running. |
Talks about extraction and nothign more.
date: 2018-08-10 | ||
--- | ||
|
||
# RFC0000: Extract CLI from Core |
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 believe this is RFC0003.
You got me Jamon! Shamelessly copied and pasted your proposal xd
…On Sat, 8 Sep 2018 at 19:16 Jamon Holmgren ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In proposals/0002-Extracted-Command-Line-Tools.md
<#13 (comment)>
:
> @@ -0,0 +1,56 @@
+---
+title: Extract CLI from Core
+author:
+- Mike Grabowski
+date: 2018-08-10
+---
+
+# RFC0000: Extract CLI from Core
I believe this is RFC0003.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#13 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWcxq1UY2jiAngmte0VW8zorBxtn3RPks5uY_txgaJpZM4V4CDa>
.
|
Are there any comments / outstanding questions before we move with this forward? I am happy to answer them. I was thinking next Wednesday could be a deadline before we move on with this one. Let me know what you think. |
No objections from my end. 👍 |
|
Here's an example of a 3rd party plugin that can be integrated into RN CLI using not-so-well-documented public API: https://github.com/grabbou/react-native-scripts/tree/master/packages/app-registry-components-to-constants |
Updated the proposal to reflect latest discussions with @axe-fb. Decided to remove notice about removing We should explore that possibility in the future, as a next step, once CLI itself is extracted. |
The last piece to figure out is your feedback on React Native CLI uses Metro inside the Right now, Metro team manually bumps Metro dependency inside React Native's package.json everytime they release a new version. My suggestion is that inside React Native CLI we would have a relaxed dependency on Metro (e.g. If there's conclusion on the right action to take, I will update the proposal and it's ready to be merged. I know there might be few unknowns here, but letting me cut a POC in two or three days once this is merged will be much easier and will shed a light on many questions. |
Is there anything preventing this from being merged? @axe-fb |
Lovely! I'll start working on it from tomorrow on! |
Right now whenever the Flow team ships a new version of Flow they have been keeping the flowconfig in the template up to date. With the CLI split into it's own repo, this will be something that the flow team will no longer be able to maintain. I can imagine a bunch of issues surrounding this point, do we have any ideas on how to solve them? |
Good point! I think what we could do is to keep the "default template"
inside React Native to make it versioned and work with whatever React
Native ships at that time. The engine itself would be kept in the CLI
repository where we would be able to iterate on it.
…On Sun, 7 Oct 2018 at 21:11 Eli White ***@***.***> wrote:
Right now whenever the Flow team ships a new version of Flow they have
been keeping the flowconfig in the template up to date. With the CLI split
into it's own repo, this will be something that the flow team will no
longer be able to maintain. I can imagine a bunch of issues surrounding
this point, do we have any ideas on how to solve them?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#13 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWcxopR4LWS4MW_pftCnNRbYoAj7ABoks5uilH1gaJpZM4V4CDa>
.
|
This proposes that we extract CLI from the core of React Native into a separate module and describes the rationale behind such a decision.
I have tried to describe as much in details as possible how I'd see this process to look like, but if you have any additional questions, let me know.
We also have an upcoming meeting regarding CLI itself this month, whoever would like to join that doesn't have an invite yet, let me know.