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

Upgrade to React Navigation 6.0 #43

Closed
Freddy03h opened this issue May 7, 2021 · 8 comments · Fixed by #47
Closed

Upgrade to React Navigation 6.0 #43

Freddy03h opened this issue May 7, 2021 · 8 comments · Fixed by #47

Comments

@Freddy03h
Copy link
Member

React Navigation 6.0 is coming! But don't worry it's still in beta On the way to React Navigation 6.0

They also wrote a really clean upgrading post that will help us migrating Upgrading from 5.x

Interesting part I noticed is changes on the Linking config. For now we don't have linking config on the bindings, I created my own on my app but never backport it here because some weird case that lead to be to tied with the app in the end.
With changes on V6 I think it will be doable, I want to test it!

This upgrade will wait Rescript Migration #42 I think.

@vknez
Copy link
Contributor

vknez commented Sep 20, 2021

In order to use React Navigation 6.x, I patched rescript-react-navigation in my project by following https://reactnavigation.org/docs/upgrading-from-5.x. I waited for you to finalize the ongoing ReScript migration, then I updated the patch to work with the latest released version.

I'm sure I haven't fixed everything :) but I think what I have would be enough for most use cases.

Would you like me to open a PR with the changes?

@MoOx
Copy link
Member

MoOx commented Sep 20, 2021

Please do. @Freddy03h did you started something already or not?

@Freddy03h
Copy link
Member Author

Not yet, I was busy on my react-native-web release ^^
I'll take a look when I'll back on the react-native side, and will test the PR on my app, thanks!

@MoOx
Copy link
Member

MoOx commented Sep 21, 2021

@Freddy03h ok nice. Thanks for your feedback. If you and @vknez are successfully using it I guess I will merge as soon as you give me your go :) Then I will update my app as soon as I can.

@Freddy03h
Copy link
Member Author

@vknez Did you also start something for the linking prop? Just to know before I try something ^^

@vknez
Copy link
Contributor

vknez commented Oct 1, 2021

@vknez Did you also start something for the linking prop? Just to know before I try something ^^

@Freddy03h No, I didn't plan to work on that in near future. In my app web routes are defined in TypeScript, so it isn't urgent.

My bigger issue right now is how to use the navigation prop and pass params to routes in a type-safe manner, and I don't see how to do it (currently we use Obj.magic just to make it work). The thing is that in React Navigation the navigation prop has different methods, depending on 1/ which navigator the screen is part of, and 2/ how that navigator is nested (different navigator functions are merged). That's pretty complicated to explain to ReScript. As for the other issue, any chance you have an example on how to pass params to routes properly (each screen has its own params)? My idea (haven't tried yet) is to have a common module that contains every screen's param types, and then pick the proper params type from there when navigating to a screen. The navigation functions, on the other hand, would be hardcoded probably. Eg. if current screen is part of a Stack navigator, which is part of a Drawer navigator, I'd have to specify different modules/functions to use with the same navigation prop: navigation->StackNavigation.pop and navigation->DrawerNavigation.openDrawer.

@MoOx MoOx closed this as completed in #47 Oct 3, 2021
@MoOx
Copy link
Member

MoOx commented Oct 3, 2021

@vknez I read real quick your comment but is this helping you https://github.com/MoOx/LifeTime/blob/05eb85efc29223e6f9340398be8e4b2c4dd65da1/src/screens/GoalsScreen.res#L65 ? If I understand correctly you are wondering how to specify navigators/params. Here if you find related module you should get your answer if I am correct :)

@Freddy03h
Copy link
Member Author

Freddy03h commented Oct 4, 2021

@vknez Ok I'll try to propose something for linking this week :)


I think the point @vknez mentioning is that routes and theirs params are barely connected.

In your example @MoOx, you can navigate to GoalEditModalScreen and omitting goalId (and passing others non-used params like newGoalType).

On my app I created a variant for my routes and custom hooks on top of react-navigation hook :

// example code

open ReactNavigation

module StackParams = {
  type params = {
    id: string,
    initialScrollVolume: option<string>,
    initialEditMode: option<bool>,
    initialVolumesChecked: option<Belt.Set.String.t>,
  }

  @obj
  external makeParams: (
    ~initialScrollVolume: string=?,
    ~initialEditMode: bool=?,
    ~initialVolumesChecked: Belt.Set.String.t=?,
    ~id: string,
    unit,
  ) => params = ""
}

include Stack.Make(StackParams)

type routes =
  | Volume(string)
  | Edition(string, option<string>, option<bool>, option<Belt.Set.String.t>)
  | Serie(string)
  | Author(string)
  | Publisher(string)

let useNavigationRoute = (route: routes): (
  option<string>,
  unit => unit,
) => {
  let navigation = ReactNavigation.Native.useNavigation()

  let (routeName, params) = switch route {
  | Volume(id) => ("volumeDetail", StackParams.makeParams(~id, ()))
  | Edition(id, initialScrollVolume, initialEditMode, initialVolumesChecked) => (
      "editionDetail",
      StackParams.makeParams(
        ~initialScrollVolume?,
        ~initialEditMode?,
        ~initialVolumesChecked?,
        ~id,
        (),
      ),
    )
  | Serie(id) => ("serieDetail", StackParams.makeParams(~id, ()))
  | Author(id) => ("authorDetail", StackParams.makeParams(~id, ()))
  | Publisher(id) => ("publisherDetail", StackParams.makeParams(~id, ()))
  }
  

  (
    None, // on my RNW app it's used to return a string for href prop
    () => navigation->Navigation.navigateWithParams(routeName, params)
  )
}

It's a custom abstract layer on top of react-navigation method. It's more than bindings, so I don't know if we plan to add helpers in react-react-navigation and how to do it.

In my case, this abstraction layer help me for sharing screens and components between react-native and react-native-web while I'm not using react-navigation on the web. (I have a Functors to create those hooks).

Maybe it could be a library on it's own, but for now, I don't know how to do it without too much boilerplate (maybe it's a bad idea).

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

Successfully merging a pull request may close this issue.

3 participants