NavigationExperimental behavior is buggy when replacing stack entries

[kushal]

Or maybe I'm misunderstanding what's supposed to happen.

Let's say my initial navigation state is
{ key: 'root', index: 0, children: [ { key: 'home' } ] }

and then I replace it with
{ key: 'root', index: 0, children: [ { key: 'settings' } ] }

As far as I can tell, NavigationScenesReducer, via NavigationAnimatedView, will keep rendering both settings and home, and since they both have the same index, compareScenes ends up sorting by alpha. Should isStale be part of compareScenes?

(Also, to be honest, I don't totally understand why stale scenes are part of the result returned by NavigationScenesReducer, it'd be great to have some more comments in the code to explain why it works this way.)

[kushal]

cc @ericvicenti

[ryankask]

I experienced the same problem yesterday and searched around and found a relevant few comments/issues:

• renderOverlay receives wrong scene after replace ericvicenti/navigation-rfc#73 - renderOverlay receives wrong scene after replace
• Support for "replace" navigation action? ericvicenti/navigation-rfc#50 -Discussion with author
• Type 'replace' not replacing view aksonov/react-native-router-flux#474
• 0.25 Support aksonov/react-native-router-flux#636 - suggests skipping 0.25 and waiting for 0.26
• aksonov/react-native-router-flux@e828fb0 - this PR addresses this issue and I implemented it in my project (it's a hack).

There are probably other GitHub issues around describing the same problem.

[dbasedow]

I reported ericvicenti/navigation-rfc#73. The stale scenes are kept around during animation. They are removed in NavigationAnimatedView._onProgressChange after the transition completed.

The issue with the same numeric index is very easy to fix (as mentioned in my other issue). After thinking about it some more, I think the best fix would be to change NavigationScenesReducer not to produce a stale scene if there already is a fresh scene at the same index with a different key.

[mmazzarolo]

I'm a newbie in NavigationExperimental and I'd like to know something more about the magic behind this:

(Also, to be honest, I don't totally understand why stale scenes are part of the result returned by NavigationScenesReducer, it'd be great to have some more comments in the code to explain why it works this way.)

I also don't get the reason behind having the scenes saved in the internal state of NavigationAnimatedView, which is causing some confusion when replacing the stack entries with a custom reducer...

[ericvicenti]

Hey folks, sorry to hear about the troubles! Also cc @hedgerwang

(Also, to be honest, I don't totally understand why stale scenes are part of the result returned by NavigationScenesReducer, it'd be great to have some more comments in the code to explain why it works this way.)
I also don't get the reason behind having the scenes saved in the internal state of NavigationAnimatedView, which is causing some confusion when replacing the stack entries with a custom reducer...

It is done in this way so that the scene stays around for the duration of the animation as the removed scene exits the screen. We keep the internal state in the AnimatedView because it is usually a bad idea to maintain the animation state in your app state.

That said, it seems like we have a bug in the scenes reducer while doing a replace. If somebody wants to debug this, write a failing test case, or add a failing UIExplorer 'replace' example, we would greatly appreciate it!

[kushal]

Ah, that's what I suspected (about keeping stale scenes for animation). I suspect one issue is around the animation being tied to index, which in this case would be from to 0 to 0, so the stale scene is never removed. I saw you assigned the bug to yourself - do you still need someone to poke around at this?

[ericvicenti]

Yeah, that's the problem. I was mistaken earlier and thought this was a problem with the scenes reducer. But you're right that this is more of a fundamental design problem. Hedger and I will work on a solution that decouples the transition from the index.

[mmazzarolo]

Hey @ericvicenti, any news regarding this issue? Thank you! 🌊

[jmurzy]

@mmazzarolo I believe @ericvicenti and @hedgerwang are working on a new version of NavigationAnimatedView called NavigationTransitionerthat's been redesigned to address the issues mentioned here. That being said, NavigationTransitioner comes with its own set of issues that need to be addressed before it can replace NavigationAnimatedView.

See jmurzy/react-router-native/issues/3

🍺

[mmazzarolo]

@jmurzy Thank you man!
In the meantime does anybody know a way to reset the stack in the current version of Navigation Experimental (@dbasedow?) or at least a way to unmount the components after a replace so that it can be re-rendered? (simply using navigationStateUtils.reset doesn't remove stale scenes)

[fdecampredon]

I created a little MonkeyPatch for my own purpose of NavigationAnimatedView that allow stack to be replaced, use it at your own risk:

// scenesReducer.js


const invariant = require('fbjs/lib/invariant');
const SCENE_KEY_PREFIX = 'scene_';

/**
 * Helper function to compare route keys (e.g. "9", "11").
 */
function compareKey(one: string, two: string): number {
    const delta = one.length - two.length;
    if (delta > 0) {
        return 1;
    }
    if (delta < 0) {
        return -1;
    }
    return one > two ? 1 : -1;
}

/**
 * Helper function to sort scenes based on their index and view key.
 */
function compareScenes(one, two) {
    if (one.index > two.index) {
        return 1;
    }
    if (one.index < two.index) {
        return -1;
    }

    return compareKey(one.key, two.key);
}

function areScenesShallowEqual(one, two): boolean {
    return (
        one.key === two.key &&
        one.index === two.index &&
        one.isStale === two.isStale &&
        one.navigationState === two.navigationState &&
        one.navigationState.key === two.navigationState.key
    );
}

export default function NavigationScenesReducer(scenes, nextState, prevState) {
    const prevScenes = new Map();
    const freshScenes = new Map();
    const staleScenes = new Map();

    // Populate stale scenes from previous scenes marked as stale.
    scenes.forEach(scene => {
        if (scene.index !== -1) {
            const { key } = scene;
            if (scene.isStale) {
                staleScenes.set(key, scene);
            }
            prevScenes.set(key, scene);
        }
    });

    const nextKeys = new Set();
    nextState.children.forEach((navigationState, index) => {
        const key = SCENE_KEY_PREFIX + navigationState.key;
        const scene = {
            index,
            isStale: false,
            key,
            navigationState,
        };
        invariant(
            !nextKeys.has(key),
            `navigationState.children[${index}].key "${key}" conflicts with` +
            'another child!'
        );
        nextKeys.add(key);

        if (staleScenes.has(key)) {
            // A previously `stale` scene is now part of the nextState, so we
            // revive it by removing it from the stale scene map.
            staleScenes.delete(key);
        }
        freshScenes.set(key, scene);
    });

    if (prevState) {
        // Look at the previous children and classify any removed scenes as `stale`.
        prevState.children.forEach((navigationState, index) => {
            const key = SCENE_KEY_PREFIX + navigationState.key;
            if (freshScenes.has(key)) {
                return;
            }
            staleScenes.set(key, {
                index,
                isStale: true,
                key,
                navigationState,
            });
        });
    }

    const nextScenes = [];

    const mergeScene = (nextScene => {
        const { key } = nextScene;
        const prevScene = prevScenes.has(key) ? prevScenes.get(key) : null;
        if (prevScene && areScenesShallowEqual(prevScene, nextScene)) {
            // Reuse `prevScene` as `scene` so view can avoid unnecessary re-render.
            // This assumes that the scene's navigation state is immutable.
            nextScenes.push(prevScene);
        } else {
            nextScenes.push(nextScene);
        }
    });

    const freshScenesArray = [...freshScenes.values()];
    staleScenes.forEach(scene => {
        if (scene.index === 0 && freshScenesArray.find(({ index }) => index === scene.index)) {
            scene.index = -1;
        }
        mergeScene(scene);
    });

    freshScenesArray.forEach(mergeScene);


    return nextScenes.sort(compareScenes);
}

// NavigationAnimatedView.js
/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 *
 */
/* eslint-disable no-underscore-dangle */

import React, { PropTypes } from 'react';
import { Animated, NavigationExperimental, StyleSheet, View } from 'react-native';
import scenesReducer from './scenesReducer';

const {
    Container: NavigationContainer,
    PropTypes: NavigationPropTypes,
} = NavigationExperimental;


function applyDefaultAnimation(position, navigationState, prevNavigationState) {
    if (navigationState.index === prevNavigationState.index) {
        position.setValue(-1);
    }
    Animated.spring(
        position,
        {
            bounciness: 0,
            toValue: navigationState.index,
        }
    ).start();
}

class NavigationAnimatedView extends React.Component {

    static propTypes = {
        applyAnimation: PropTypes.func,
        navigationState: NavigationPropTypes.navigationState.isRequired,
        onNavigate: PropTypes.func.isRequired,
        renderOverlay: PropTypes.func,
        renderScene: PropTypes.func.isRequired,
        style: View.propTypes.style,
    };

    static defaultProps = {
        applyAnimation: applyDefaultAnimation,
    };

    state = {
        layout: {
            height: new Animated.Value(0),
            initHeight: 0,
            initWidth: 0,
            isMeasured: false,
            width: new Animated.Value(0),
        },
        position: new Animated.Value(this.props.navigationState.index),
        scenes: scenesReducer([], this.props.navigationState),
    }

    componentDidMount() {
        this._positionListener = this.state.position.addListener(this._onProgressChange);
    }

    componentWillReceiveProps(nextProps) {
        if (nextProps.navigationState !== this.props.navigationState) {
            this.setState({
                scenes: scenesReducer(
                    this.state.scenes,
                    nextProps.navigationState,
                    this.props.navigationState
                ),
            });
        }
    }

    componentDidUpdate(lastProps) {
        const { index: lastIndex, children: lastChildren } = lastProps.navigationState;
        const { index, children } = this.props.navigationState;
        if (
            lastIndex !== index || (
                index === 0 && 
                children && lastChildren && 
                children.length && lastChildren.length &&
                children[0].key !== lastChildren[0].key
            )
        ) {
            this.props.applyAnimation(
                this.state.position,
                this.props.navigationState,
                lastProps.navigationState
            );
        }
    }

    componentWillUnmount() {
        this.state.position.removeListener(this._positionListener);
    }

    _onProgressChange = (data: Object) => {
        const delta = Math.abs(data.value - this.props.navigationState.index);
        if (delta > Number.EPSILON) {
            return;
        }

        const scenes = this.state.scenes.filter(scene => !scene.isStale);

        if (scenes.length !== this.state.scenes.length) {
            this.setState({ scenes });
        }
    }

    _onLayout = (event) => {
        const { height, width } = event.nativeEvent.layout;

        const layout = {
            ...this.state.layout,
            initHeight: height,
            initWidth: width,
            isMeasured: true,
        };

        layout.height.setValue(height);
        layout.width.setValue(width);

        this.setState({ layout });
    }

    _renderScenes() {
        return this.state.scenes.map(this._renderScene, this);
    }

    _renderScene(scene) {
        const {
            navigationState,
            onNavigate,
            renderScene,
        } = this.props;

        const {
            position,
            scenes,
        } = this.state;

        return renderScene({
            layout: this.state.layout,
            navigationState,
            onNavigate,
            position,
            scene,
            scenes,
        });
    }

    _renderOverlay() {
        if (this.props.renderOverlay) {
            const {
                navigationState,
                onNavigate,
                renderOverlay,
            } = this.props;

            const {
                position,
                scenes,
            } = this.state;

            return renderOverlay({
                layout: this.state.layout,
                navigationState,
                onNavigate,
                position,
                scene: scenes[navigationState.index],
                scenes,
            });
        }
        return null;
    }


    render() {
        const overlay = this._renderOverlay();
        const scenes = this._renderScenes();
        return (
            <View
              onLayout={this._onLayout}
              style={this.props.style}>
                <View style={styles.scenes} key="scenes">
                    {scenes}
                </View>
                {overlay}
            </View>
        );
    }
}

const styles = StyleSheet.create({
    scenes: {
        flex: 1,
    },
});

export default NavigationContainer.create(NavigationAnimatedView);