seantempesta / cljs-react-navigation

CLJS wrappers for react-navigation
MIT License
67 stars 23 forks source link

Navigator function names are deprecated in react-navigation 2 #13

Open sulami opened 6 years ago

sulami commented 6 years ago

When upgrading react-navigation to 2.0, I'm getting a lot of "The StackNavigator function name is deprecated, please use createStackNavigator instead", which is fixed easily enough by something like this.

(src/clsjs_react_navigation/base.cljs)

 ;; Navigators
 (defonce createNavigator (gobj/get ReactNavigation #js ["createNavigator"]))
-(defonce StackNavigator (gobj/get ReactNavigation #js ["StackNavigator"]))
+(defonce StackNavigator (gobj/get ReactNavigation #js ["createStackNavigator"]))
 (defonce TabNavigator (gobj/get ReactNavigation #js ["TabNavigator"]))
+(defonce BottomTabNavigator (gobj/get ReactNavigation #js ["createBottomTabNavigator"]))
+(defonce MaterialTabNavigator (gobj/get ReactNavigation #js ["createMaterialTabNavigator"]))
-(defonce DrawerNavigator (gobj/get ReactNavigation #js ["DrawerNavigator"]))
-(defonce SwitchNavigator (gobj/get ReactNavigation #js ["SwitchNavigator"]))
+(defonce DrawerNavigator (gobj/get ReactNavigation #js ["createDrawerNavigator"]))
+(defonce SwitchNavigator (gobj/get ReactNavigation #js ["createSwitchNavigator"]))

 ;; Routers
-(defonce StackRouter (gobj/get ReactNavigation #js ["StackRouter"]))
-(defonce TabRouter (gobj/get ReactNavigation #js ["TabRouter"]))
+(defonce StackRouter (gobj/get ReactNavigation #js ["createStackRouter"]))
+(defonce TabRouter (gobj/get ReactNavigation #js ["createTabRouter"]))

TabNavigator has actually been split into createBottomTabNavigator and createMaterialTopTabNavigator, so it probably makes sense to add these as seperate bindings.

TabView has been moved to https://github.com/react-navigation/react-navigation-tabs, which I guess makes sense to require instead of having it be an optional dependency.

 (defonce React (js/require "react"))
 (defonce ReactNavigation (js/require "react-navigation"))
+(defonce ReactNavigationTabs (js/require "react-navigation-tabs"))
 (defonce isValidElement (gobj/get React #js ["isValidElement"]))
 ;; Views
 (defonce Transitioner (gobj/get ReactNavigation #js ["Transitioner"]))
 (defonce CardStack (gobj/get ReactNavigation #js ["CardStack"]))
 (defonce DrawerView (gobj/get ReactNavigation #js ["DrawerView"]))
-(defonce TabView (gobj/get ReactNavigation #js ["TabView"]))
+(defonce TabView (gobj/get ReactNavigationTabs #js ["TabView"]))

-(assert (and React ReactNavigation) "React and React Navigation must be installed.  Maybe NPM install them and restart the packager?")
+(assert (and React ReactNavigation ReactNavigationTabs) "React, React Navigation and React Navigation Tabs must be installed.  Maybe NPM install them and restart the packager?")

I'm not sure if there are any other changes required, I haven't actually tested most of the functions, but it builds in my project without warnings (which only uses StackNavigator/StackRouter). I can prepare a PR for that if you want me to.

dotemacs commented 6 years ago

Hello @sulami

do you want to create a pull request and at least we can have it ready, because from your diffs above it seems like you have most of this.

Maybe we can have two branches, version 1.x tracking the changes that are present in react-navigation 1.x and 2.x tracking the changes that are present in react-navigation 2.x? What do you think?

Thanks for your time, looking forward to the PR!

seantempesta commented 6 years ago

Hey @sulami: Maybe we should just create a new namespace? That way we don't break existing behavior and we can define new specs/functions to operate in parallel?

Also, I'm super busy at the moment. If you have any interest in working on this project I'm happy to give you admin access to check new changes in and merge pull requests.

sulami commented 6 years ago

I can see whether I can whip something up some time this or next week and submit it for a look-over to make sure it’s sensible.

--

Robin Schroer On 31 May 2018, 17:26 +0100, Sean Tempesta notifications@github.com, wrote:

Hey @sulami: Maybe we should just create a new namespace? That way we don't break existing behavior and we can define new specs/functions to operate in parallel? Also, I'm super busy at the moment. If you have any interest in working on this project I'm happy to give you admin access to check new changes in and merge pull requests. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

madstap commented 6 years ago

+1 for creating a new namespace to avoid breakage

madstap commented 6 years ago

There's also this that changed between 1 and 2 https://github.com/react-navigation/react-navigation/issues/3930

sulami commented 6 years ago

Where would you like to have that separate namespace located ideally?

jamesnyika commented 5 years ago

FYI.. anyone looking to use v3 - I have a way to do it with shadow-cljs. React Navigation in Clojurescript

dotemacs commented 5 years ago

Thanks for looking into this @jamesnyika.

What do others think about this?

davidpham87 commented 5 years ago

I think the solution offered by cljs-react-navigation is more robust in the sense that we are used to dispatch events in re-frame (and switching screen is an event).

jamesnyika commented 5 years ago

@davidpham87 There is actually nothing preventing you from doing this yourself in re-frame. When I handle navigation in my re-frame apps (including in react native) I have a single navigation event handler/interceptor. It is usually the last one to run in a pipeline of event handlers. It takes information from the event to determine where to navigate to.

(reg-event-db
 :navigation/event
 (fn [db [_ {:keys [event data]}]]

    ;; event handler receives navigation props and another bunch of data I send in a supporting map
   (let [navigation (:navigation/handle db)
         navigation-map (:navigation/map db)

        ;; destructure the js navigation props to get the navigate function
         {:keys [navigate goBack]} (util/keywordize navigation)]

      ;;call navigation simply
     ((js->clj navigate) (event @e/navigation-map))

      ;;do not forget to return the db inside an event handler (or update it first if you need to) 
     db)))

The only reason I think to roll your own events doing this is to reduce one more library dependency for something that you is really inexpensive to design and allows you to retain understanding.

davidpham87 commented 5 years ago

@jamesnyika

Thanks for your answer. My issue is we pollute the component with additional boilerplate that did not feel natural (coming from web development).

I end up using something like this:

(ns my.app.event
  (:require
   [re-frame.core :as rf :refer [reg-event-fx reg-event-db reg-fx]]
   ["react-navigation" :as rnav]))

(reg-event-db
 :set-navigation
 (fn [db [_ nav]]
   (assoc db :navigator nav)))

(reg-fx
 :active-screen
 (fn [[navigator screen]]
   (when navigator
     (.dispatch navigator
                (.navigate rnav/NavigationActions
                           #js {:routeName screen})))))

(reg-event-fx
 :register-active-screen
 (fn [{db :db} [_ screen params]]
   {:db (assoc db :active-screen screen
               :active-screen-params (if params params {}))}))

(reg-event-fx
 :set-active-screen
 (fn [{db :db} [_ screen-id]]
   {:db (assoc db :active-screen screen-id)
    :active-screen [(db :navigator) screen-id]}))

(reg-fx
 :navigation-drawer-action
 (fn [[navigator action]]
   (when navigator
     (.dispatch navigator
                (case action
                  :toggle (.toggleDrawer rnav/DrawerActions)
                  :open (.openDrawer rnav/DrawerActions)
                  :close (.closeDrawer rnav/DrawerActions))))))

(reg-event-fx
 :set-drawer-state
 (fn [{db :db} [_ action]]
   {:db db
    :navigation-drawer-action [(db :navigator) action]}))

(reg-event-fx
 :toggle-drawer
 (fn [{db :db} [_ action]]
   {:db db
    :navigation-drawer-action [(db :navigator) :toggle]}))

And in main

(defn root []
      [:> app-container {:ref #(rf/dispatch [:set-navigation %])}])
jamesnyika commented 5 years ago

@davidpham87 this is good. .let me digest..

jamesnyika commented 5 years ago

@davidpham87 I think I see what you are up to here. And it is a perfectly legitimate approach. One feature of the single database approach that re-frame has is that it is really simple (one map or db to store everything). As soon as you start building a business application thought, you almost want a way to separate out business data from data that supports the running application (typically session information, current screen, nav etc) that have nothing to do with the business domain.

I only took the approach I have so that I can use a transformation stream to clean out the business data as it heads to its datascript database, while the rest of the system state can remain in the Out-of-box re-frame map.

Well.. different routes: eternal complexity!