From 579cebc53a1efd0006636ab5844259b57edaaecc Mon Sep 17 00:00:00 2001 From: Satyajit Sahoo <satyajit.happy@gmail.com> Date: Mon, 29 Jan 2018 00:47:29 +0100 Subject: [PATCH 1/2] Create 0002-deprecate-screen-props.md --- 0002-deprecate-screen-props.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 0002-deprecate-screen-props.md diff --git a/0002-deprecate-screen-props.md b/0002-deprecate-screen-props.md new file mode 100644 index 0000000..1daee37 --- /dev/null +++ b/0002-deprecate-screen-props.md @@ -0,0 +1,33 @@ +# What is `screenProps` + +The `screenProps` prop allows the user to pass a prop to a navigator's children screen. This prop propagates deep into the children screens. + +```js +<RootNavigator screenProps={{ foo: 'bar' }} /> +``` + +# Use case + +I initially added this prop to be able to pass some data to the child screens in a navigator from the parent. People are also using it to pass all kinds of stuff, such as global state, functions, reference to `this` etc. + +# Problems + +Apart from the misuse, it's too easy to de-optimize the whole tree with `screenProps`. For example, `screenProps={{ foo: 'bar' }}` creates a new object every render, which breaks `shouldComponentUpdate` in all children navigators and screens, no matter how deep the tree is, and will re-render everything for a simple change. And actually most of the usage I have seen in the bug reports use it this way. It can also cause issues like this https://github.com/react-navigation/react-navigation/issues/2625 + +Even if we can document to avoid the re-render issue, this prop was present due to lack of other ways to achieve the same functionality. Now that React's context API is finally going to be stable, it's no longer the case. + +# Alternatives + +## Dynamically passing params + +My initial use case was passing some data to the child screens, which can be addressed by allowing to pass params dynamically. + +```js +<Tabs initialParams={{ foo: 'bar' }} /> +``` + +Above code also has the re-render issue, but localized to one navigator. And the name `params` makes it clear that it's for passing params and not arbitrary data. + +## Context API + +React's context is a more explicit way to achieve the same functionality which doesn't interfere with `shouldComponentUpdate`. When using context, only the components using the data passed from the top will re-render, and not unrelated screens. From 847187e578d9a0350f31f10171deaeb08affaf46 Mon Sep 17 00:00:00 2001 From: Satyajit Sahoo <satyajit.happy@gmail.com> Date: Sun, 11 Feb 2018 14:20:08 +0100 Subject: [PATCH 2/2] Update 0002-deprecate-screen-props.md --- 0002-deprecate-screen-props.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/0002-deprecate-screen-props.md b/0002-deprecate-screen-props.md index 1daee37..d59cce7 100644 --- a/0002-deprecate-screen-props.md +++ b/0002-deprecate-screen-props.md @@ -8,11 +8,15 @@ The `screenProps` prop allows the user to pass a prop to a navigator's children # Use case -I initially added this prop to be able to pass some data to the child screens in a navigator from the parent. People are also using it to pass all kinds of stuff, such as global state, functions, reference to `this` etc. +I initially added this prop to be able to pass some data to the child screens in a navigator from the parent. There are several other ways people are using this: + +1. State management, to pass state from root to some screen down the tree +2. Reference to parent component instance, methods etc. # Problems -Apart from the misuse, it's too easy to de-optimize the whole tree with `screenProps`. For example, `screenProps={{ foo: 'bar' }}` creates a new object every render, which breaks `shouldComponentUpdate` in all children navigators and screens, no matter how deep the tree is, and will re-render everything for a simple change. And actually most of the usage I have seen in the bug reports use it this way. It can also cause issues like this https://github.com/react-navigation/react-navigation/issues/2625 +1. People misuse this as a way to solve state-management issues, which is not something a navigation library needs to solve +2. Apart from the misuse, it's too easy to de-optimize the whole tree with `screenProps`. For example, `screenProps={{ foo: 'bar' }}` creates a new object every render, which breaks `shouldComponentUpdate` in all children navigators and screens, no matter how deep the tree is, and will re-render everything for a simple change. And actually most of the usage I have seen in the bug reports use it this way. It can also cause issues like this https://github.com/react-navigation/react-navigation/issues/2625 Even if we can document to avoid the re-render issue, this prop was present due to lack of other ways to achieve the same functionality. Now that React's context API is finally going to be stable, it's no longer the case.