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

Split this repo in 2: react and react-native #43

Open
MangelMaxime opened this issue Jul 3, 2019 · 8 comments
Open

Split this repo in 2: react and react-native #43

MangelMaxime opened this issue Jul 3, 2019 · 8 comments
Assignees

Comments

@MangelMaxime
Copy link
Member

Description

We are introducing a new tool in Fable ecosystem called Femto.

The goal of this tool is to help people check if they have the required npm packages installed.

However, we are facing a problem with Fable.Elmish.React because it's using:

  • react-dom API exposed from Fable.React: source
  • react-native API by importing it locally via interop: source

But we don't want the user to install react-native if they are targetting only the browser.

@et1975 Would you agree to split this repo between Fable.Elmish.React and Fable.Elmish.ReactNative?

For info, we are also planning to strip Fable.React from ReactDom API in order to have this tree of deps. And so if you agree, we will prepare up front the Fable.React/ Fable.ReactDom split and then work on Fable.Elmish.React split.

@Zaid-Ajaj
Copy link
Member

The problem with having this repo support both react-dom and react-native is that the functions withReactNative and withReact(Synchronous | Batched) will be available for users of Fable.Elmish.React regardless of the target runtime (browser vs. mobile)

The split will make the dependency hierarchy correct, allowing for tools like Femto to resolve the dependencies automatically without the user having to look up and manually install the npm packages required

@et1975
Copy link
Member

et1975 commented Jul 3, 2019

Makes sense 👍

@MangelMaxime
Copy link
Member Author

I am working on the repo split and face a decision.

In this repo, we have a Common.fs files which provide a react component (lazyView) which can be used from both react-dom or native-native I think.


Should we create three repos?

  • Fable.Elmish.React
  • Fable.Elmish.React.Dom
  • Fable.Elmish.React.Native

Where Fable.Elmish.React contains only the Common.fs code and the other packages contain the Program.XXX API specific to their environment target.

Should we duplicate the Common.fs between both repo?

Should we finally deprecate lazyView in favour of React memo, etc. as it has been discussed once?


Personally, I am in favour of creating 3 repos or deprecating the lazyView.

@Zaid-Ajaj
Copy link
Member

I feel like separation of 3 repo's is a bit overkill: why not simply add Common.fs in both libraries of Fable.Elmish.React and Fable.Elmish.ReactNative (without the dot) where Fable.Elmish.React is the default react implementation for the browser and relies on Fable.ReactDom (which depends on Fable.React),

@MangelMaxime
Copy link
Member Author

Because code duplication is never a good thing especially if we can have a simple solution. But it's indeed a solution.

Also, having 3 packages follows react package organisation and makes everything explicit.

I prefer to have several opinions on this subject as I don't want to do the split/breaking change several times.

@et1975
Copy link
Member

et1975 commented Jul 24, 2019

I expected this would come up, here are some thoughts:

  • From our own reuse perspective I'm not worried about duplicating the code.
  • From consumer (views) reuse perspective I think we're OK to duplicate if the user does source linking, although someone should look at ReactXP and make sure.
  • Since the objective is to establish strong relationship with JS dependencies, IMO mirroring React's dependencies hierachy would justify creating 3 libs on our side.
  • Having to include 2 dependencies would be annoying to the user, but React went through with it and since we have project templates I think we're OK.

@MangelMaxime
Copy link
Member Author

From consumer (views) reuse perspective I think we're OK to duplicate if the user does source linking, although someone should look at ReactXP and make sure.

I don't really understand how ReactXP works (didn't set up a whole project for it). But by looking at the example and documentation it's a separate library from react.

They seem to require the user to include both reactpx and react as a dependency in their project.

Since the objective is to establish strong relationship with JS dependencies, IMO mirroring React's dependencies hierachy would justify creating 3 libs on our side.

I agree

Having to include 2 dependencies would be annoying to the user, but React went through with it and since we have project templates I think we're OK.

I need to check but because of Fable.Elmish.React being a transitive dependency of Fable.Elmish.React.Dom does the user needs to include Fable.Elmish.React himself?

I would say the safe move is to follow the JS dependencies.

@Zaid-Ajaj
Copy link
Member

I need to check but because of Fable.Elmish.React being a transitive dependency of Fable.Elmish.React.Dom does the user needs to include Fable.Elmish.React himself?

No, Fable.Elmish.React will be downloaded automatically if it is a dependency of Fable.Elmish.React.Dom and the user only installs Fable.Elmish.React.Dom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants