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

Circular Reference Issue in StackNavigator? #1396

Closed
MovingGifts opened this issue May 5, 2017 · 12 comments
Closed

Circular Reference Issue in StackNavigator? #1396

MovingGifts opened this issue May 5, 2017 · 12 comments
Assignees

Comments

@MovingGifts
Copy link

export const InstructorProfileStack = StackNavigator({
    InstructorProfile: {
      screen: InstructorProfile,
    },
    Schedule: {
      screen: Schedule,
    },
    Courses: {
      screen: CoursesStack,  // Circular Reference 1
    },
  },
  {
    headerMode: 'screen',
    navigationOptions: {
      headerVisible: false
    }
});

export const CoursesStack = StackNavigator({
    Courses: {
      screen: Courses,
    },
    StudentForm: {
      screen: StudentForm,
    },
    InstructorProfile: {
      screen: InstructorProfileStack,  // Circular Reference 2
    },
  },
  {
    headerMode: 'screen',
    navigationOptions: {
      headerVisible: false
    }
});

InstructorProfileStack should be able to navigate to CoursesStack and CoursesStack should be able to navigate to InstructorProfileStack, each within their respective StackNavigator stack.

The issue is that // Circular Reference 1, CoursesStack is not yet defined, but if I swap the order, the other one will not be defined. So how can we get them both to navigate correctly regardless of their defined order?

@matthamil
Copy link
Member

One possible solution is to put all of your possible stack screens into one generic file called Stack.js like so:

// import components at the top as usual

export const Stack = {
  Feed: {
    name: 'Feed',
    screen: FeedContainer
  },
  Home: {
    name: 'Home',
    screen: HomeContainer
  },
  Messaging: {
    name: 'Messaging',
    screen: MessagingContainer
  },
  Notifications: {
    name: 'Nofitications',
    screen: NotificationsContainer
  },
  Settings: {
    name: 'Settings',
    screen: SettingsContainer
  },
  UserProfile: {
    name: 'User Profile',
    screen: UserProfileContainer
  }
};

Then you can define InstructorProfileStack like so:

import { Stack } from './Stack';

export const InstructorProfileStack = StackNavigator({
    ...Stack
  },
  {
    headerMode: 'screen',
    navigationOptions: {
      headerVisible: false
    }
});

and do the same for CoursesStack. The reason you could do this is so both stacks have access to any screen. This may or may not be a viable solution for you.

An alternative approach (and probably better) is to wrap your two navigators in another navigator:

export const StackWrapper = StackNavigator({
  InstructorProfile: {
    // routes
  },
 Courses: {
    // routes
 },
 { /* stackNavigatorConfig */ }
}

@MovingGifts
Copy link
Author

Thanks for your response @matthamil , but I think it should be simpler than that implementation of just creating more and more navigators since we already defined the navigators we need.

I think this circular reference scenario can occur many times, and I was thinking ideally something like the syntax I have should work, but it doesn't due to JS order of execution.

GraphQL also has the same issue, but their engineers' solution to this circular reference problem is quite simple, turn those fields into functions. So something like this should work:

export const InstructorProfileStack = StackNavigator({
    InstructorProfile: {
      screen: InstructorProfile,
    },
    Schedule: {
      screen: Schedule,
    },
    Courses: () => ({
      screen: CoursesStack,  // Circular Reference Function 1
    }),
  },
  {
    headerMode: 'screen',
    navigationOptions: {
      headerVisible: false
    }
};

export const CoursesStack = StackNavigator({
    Courses: {
      screen: Courses,
    },
    StudentForm: {
      screen: StudentForm,
    },
    InstructorProfile: () => ({
      screen: InstructorProfileStack,  // Circular Reference Function 2
    }),
  },
  {
    headerMode: 'screen',
    navigationOptions: {
      headerVisible: false
    }
});

The router.js file with those functions should now have access to the screens, so we don't get the Route 'Courses' should declare a screen error anymore.

I tried the above but it also doesn't work with this library.

What's the cleanest solution? Is it possible to implement a simple, clean way to solve the react-navigation circular reference issue like the GraphQL engineers, via functions?
@ericvicenti @grabbou @satya164

@satya164
Copy link
Member

satya164 commented May 5, 2017

This is currently undocumented, but you can do getScreen: () => CoursesStack instead of screen: CoursesStack

@satya164 satya164 closed this as completed May 5, 2017
@MovingGifts
Copy link
Author

@satya164 I really thought that'll work:

Courses: {
  getScreen: () => CoursesStack,  // Circular Reference Function 1
},

But it behaves the same way:

simulator screen shot may 5 2017 8 13 43 pm

It doesn't consider the CoursesStack a valid navigator! This is however the sort of syntax I am looking for, a simple function. Is there anyway to get it to recognize it as a valid navigator (since it is available in the same file, just the "order of operations" is the issue)?

@satya164
Copy link
Member

satya164 commented May 6, 2017

Looks like a bug. It shouldn't try to validate this as declaration time as it's supposed to be lazy loaded.

cc @ericvicenti

@satya164 satya164 reopened this May 6, 2017
@MovingGifts
Copy link
Author

bump @ericvicenti

@MovingGifts
Copy link
Author

@ericvicenti?

@MovingGifts
Copy link
Author

@skevy can this be looked at, it seems like an ignored bug?

@matthamil
Copy link
Member

@MovingGifts There are currently 417 open issues and only so many maintainers :) it might take them a while until they can take actions to resolve this. They are always appreciative of PRs though.

@ghost
Copy link

ghost commented Oct 6, 2017

I don't think getScreen is the answer as the docs says

// Note: Child navigators cannot be configured using getScreen because
// the router will not be accessible. Navigators must be configured
// using screen: MyNavigator

https://github.com/react-community/react-navigation/blob/master/docs/api/routers/StackRouter.md#routeconfig

@MovingGifts
Copy link
Author

Yeah it doesn't really work. I know GraphQL solved the issue really well as I said in my comments up there, and judging by @satya164 tags it shows that it's something this library should have. I will bring it up to the new list @matthamil made.

@brentvatne
Copy link
Member

can someone who would like to see this issue resolved please create a new issue with a reproducible example on https://snack.expo.io or in a github repository, with a minimal amount of superfluous code? (please don't post your entire app, just a small one to reproduce the problem). thanks!

@react-navigation react-navigation locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants