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

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

Open
sulami opened this issue May 30, 2018 · 13 comments
Open

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

sulami opened this issue May 30, 2018 · 13 comments

Comments

@sulami
Copy link

sulami commented May 30, 2018

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
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Author

sulami commented Jun 4, 2018 via email

@madstap
Copy link

madstap commented Jun 5, 2018

+1 for creating a new namespace to avoid breakage

@madstap
Copy link

madstap commented Jun 5, 2018

There's also this that changed between 1 and 2 react-navigation/react-navigation#3930

@sulami
Copy link
Author

sulami commented Jun 27, 2018

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

@jamesnyika
Copy link

jamesnyika commented May 20, 2019

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

@dotemacs
Copy link
Collaborator

Thanks for looking into this @jamesnyika.

What do others think about this?

@davidpham87
Copy link

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
Copy link

@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
Copy link

davidpham87 commented Jul 7, 2019

@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
Copy link

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

@jamesnyika
Copy link

@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!

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

6 participants