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

useMachine stops working after fast refresh #995

Closed
Nohac opened this issue Feb 12, 2020 · 26 comments
Closed

useMachine stops working after fast refresh #995

Nohac opened this issue Feb 12, 2020 · 26 comments

Comments

@Nohac
Copy link

Nohac commented Feb 12, 2020

Description
The useMachine hook works as expected in react native when the app loads for the first time, then after a "fast reload", the hook stops updating the state, send still works and the machine completes actions in the background, but the state is never updated.

Expected Result
When performing a "fast reload", the useMachine hook should still return the current state.

Actual Result
After "fast reload" the useMachine hook never updates even though the state machine is still working.

Reproduction
Should be fairly easy to reproduce using react native and performing a fast refresh.

Additional context
xstate: 4.7.8
xstate/react: 0.8.1
react: 16.9.0
react-native: 0.61.5

@Nohac Nohac added the bug label Feb 12, 2020
@Andarist
Copy link
Member

I'm afraid that we are not React Native developers on a daily basis and fast refresh is not yet available on the web, so we don't quite know how it works exactly and what kind of constraints does it impose. From what I understand it should "just work", but apparently it doesn't. Could you dig into this deeper to provide us more information about the problem?

@Nohac
Copy link
Author

Nohac commented Feb 14, 2020

So I ended up trying out the hook that was used in this youtube video https://youtu.be/73Ch_EL4YVc
I rewrote it to typescript, and that ended up working fine. Here's the code for anyone interested:

import React, { useEffect, useMemo, useState } from 'react';
import {
  EventObject,
  interpret,
  Machine,
  StateMachine,
  Typestate,
  State,
  Interpreter,
} from 'xstate';

export const useMachine = <
  TContext,
  TEvent extends EventObject,
  TTypestate extends Typestate<TContext> = any
>(
  machine: StateMachine<TContext, any, TEvent, TTypestate>,
): [
  State<TContext, TEvent, any, TTypestate>,
  Interpreter<TContext, any, TEvent, TTypestate>['send'],
] => {
  const [current, setCurrent] = useState(machine.initialState);

  const service = useMemo(
    () =>
      interpret(machine)
        .onTransition(state => {
          if (state.changed) {
            setCurrent(state);
          }
        })
        .start(),
    [],
  );

  useEffect(() => {
    return () => service.stop() as any;
  }, []);

  return [current, service.send];
};

My biggest suspicion lies with the use of useConstant instend of useMemo, but I haven't been able to confirm that yet.

@Andarist
Copy link
Member

Andarist commented Feb 14, 2020

My biggest suspicion lies with the use of useConstant instend of useMemo

That's also my suspicion, but would be great to confirm it and to actually understand why this happens. From what I understand Fast Refresh tries to "sync" states before & after refresh and I would expect a similar thing to happen for refs, and if this cannot be done Fast Refresh should cause a full reload in such a case (its the fallback behavior I believe). This technique (useConstant) is mentioned in React docs (albeit without useConstant abstraction) so I would expect that Fast Refresh should handle this case fine.

Could you maybe dive into your node_modules and change useConstant call to:

const [returnValue] = React.useState(() => { /* useConstant initializer */ })

And of course, check if this changes anything?

EDIT:// Are you sure you are using @xstate/react version with useConstant used? You have mentioned using 0.8.1 which doesn't have this hook in it.

@Nohac
Copy link
Author

Nohac commented Feb 14, 2020

You're completely right, I don't have the useConstant version, sorry about that. I only looked at the code on github since "go to definition" only takes me to the index.d.ts file.
So it's possible that the next release will work, then.. What is the easiest way to test the new changes?

@Andarist
Copy link
Member

yarn add @xstate/react@1.0.0-rc.3

@Nohac
Copy link
Author

Nohac commented Feb 15, 2020

Same issue with that version, however, it gave a warning this time:

Machine given to `useMachine` has changed between renders. This is not
supported and might lead to unexpected results.  Please make sure that you pass
the same Machine as argument each time.

And when trying to send an event after fast refresh:

Warning: Event "LOGIN" was sent to stopped service "(machine)". This service
has already reached its final state, and will not transition.

Update: Hot-patching the library and replacing useConstant with useState seems to have solved the problem.

@Andarist
Copy link
Member

Could you paste in the code of your component?

@Nohac
Copy link
Author

Nohac commented Feb 15, 2020

This is essentially what my code looks like, but simplified.
Tested with @xstate/react@1.0.0-rc.3, same problem.

import React from 'react';
import { Text, View, Button } from 'react-native';
import { useMachine } from '@xstate/react';
import { Machine } from 'xstate';

interface BootStateSchema {
  states: {
    authenticate: {};
    login: {};
  };
}

interface BootContext {}

type BootEvent = { type: 'LOGIN' };

const myMachine = Machine<BootContext, BootStateSchema, BootEvent>({
  initial: 'login',
  context: {},
  states: {
    authenticate: {
      invoke: {
        src: async () => new Promise(resolve => setTimeout(resolve, 1000)),
        onDone: {
          target: 'login',
        },
      },
    },
    login: {
      on: {
        LOGIN: {
          target: 'authenticate',
        },
      },
    },
  },
});

function App() {
  const [current, send] = useMachine(myMachine);

  return (
    <View>
      <Text>Current state: {current.value}</Text>
      <Button onPress={() => send('LOGIN')} title={'Login'} />
    </View>
  );

}

@Andarist
Copy link
Member

Ok, i think the whole module reloads so myMachine gets recreates - which is not good for us. Could you create a repro case for this using expo? I would love to take a look at how exactly this behaves and having a repro case would allow me to jump into this quicker

@CodingDive
Copy link
Contributor

CodingDive commented Feb 16, 2020

This isn't limited to the fast refresh of React native. I've been using fast-refresh in my CRA project for quite a while now using https://github.com/esetnik/customize-cra-react-refresh. I had to manually refresh the browser on every change as the machines stopped handling events. That being said, the state was always preserved correctly.

@Andarist if you would you rather debug this with normal React, I can create a CRA fast refresh Codesandbox for you? 😊

@Andarist
Copy link
Member

@CodingDive that would be sweat!

@CodingDive
Copy link
Contributor

CodingDive commented Feb 18, 2020

@Andarist https://github.com/CodingDive/xstate-cra-fast-refresh-example didn't work within a Codesandbox probably because their templates are still on CRA v1. It has a readme with instructions on how to replicate the issue. Note that it currently still uses @xstate/react v0.8.1.

@Nohac can you confirm that this is the same error you were getting in React native?

@Nohac
Copy link
Author

Nohac commented Feb 18, 2020

@CodingDive It's the same as I'm getting with react native. I also updated and tested with 1.0.0-rc.3 which gives me the same warnings as I mentioned earlier.

@idlefingers
Copy link

I'm having the same problem in next.js now they've started using fast-refresh. Here's a tiny example of it breaking using next: https://github.com/idlefingers/next-xstate-fast-refresh-bug

Hope that helps!

@Andarist
Copy link
Member

I'm too familiar with Fast Refresh design, does anyone have a link to its technical details?

I've investigated your repro (thanks for it!) with our @xstate/react@next (so a different one that you have installed there). What I'm observing is that our effect gets "remounted" - its cleanup (service.stop()) is being called even though [] is passed as inputs so it should only be cleaned up when unmounting. However, I've seen the React team mentioning that in the future this might change and effects could potentially "remount" like this at different occasions as well. How this will work is still an unknown to me so I wouldn't like to plan ahead for it too much, unless we would be able to figure this out quickly.

So the problem here is that we stop the machine when fast refresh happens - things like state & refs are being preserved as the whole component is not being remounted. I'm honestly not sure what we should do here - we could try to restart stopped machine by supplying initial state to it (which we should be able to read from the stopped service), but this really sounds like a can of worm. We ideally would like to have our effect not being remounted as we'd like to have a guarantee that its lifetime (and thus our machine's) is exactly the same as the component's lifetime.

@hnordt
Copy link
Contributor

hnordt commented May 30, 2020

@Andarist I've changed the useConstant to:

  const latestStateRef = React.useRef(null)

  const service = React.useMemo(() => {
    const machineConfig = {
      context,
      guards,
      actions,
      activities,
      services,
      delays,
    }

    const createdMachine = machine.withConfig(machineConfig, {
      ...machine.context,
      ...context,
    })

    return interpret(createdMachine, interpreterOptions)
      .start(
        rehydratedState
          ? State.create(rehydratedState)
          : latestStateRef.current
          ? State.create(latestStateRef.current)
          : undefined
      )
      .onTransition((state) => {
        latestStateRef.current = state
      })
  }, [])

It worked like a charm. latestStateRef was used to enable rehydration.


Here's the full hook if you want to test:

import React from "react"
import { State, interpret } from "xstate"

export default function useMachine(machine, options = {}) {
  const {
    state: rehydratedState,
    context,
    guards,
    actions,
    activities,
    services,
    delays,
    ...interpreterOptions
  } = options

  let latestStateRef = React.useRef(null)

  const service = React.useMemo(
    () =>
      interpret(
        machine.withConfig(
          {
            context,
            guards,
            actions,
            activities,
            services,
            delays,
          },
          {
            ...machine.context,
            ...context,
          }
        ),
        interpreterOptions
      )
        .start(
          rehydratedState
            ? State.create(rehydratedState)
            : latestStateRef.current
            ? State.create(latestStateRef.current)
            : undefined
        )
        .onTransition((state) => (latestStateRef.current = state)),
    []
  )

  const [state, setState] = React.useState(service.state)

  React.useEffect(() => {
    service.onTransition((state) => {
      if (state.changed) {
        setState(state)
      }
    })

    setState(service.state)

    return () => service.stop()
  }, [])

  React.useEffect(() => {
    Object.assign(service.machine.options.actions, actions)
  }, [actions])

  React.useEffect(() => {
    Object.assign(service.machine.options.services, services)
  }, [services])

  return [state, service.send, service]
}

@Andarist
Copy link
Member

The problem with useMemo is that it is not a semantic guarantee - but is only treated as an optimization. Theoretically React reserves the right for itself to just drop the cached value and reevaluate useMemo at any given time. This means that a previously created service could not be stopped and you would start its copy.

There is also a problem with this that we trust that currently received machine can be rehydrated with the latestStateRef.current - but one could edit their machine config completely which would lead to fast refresh kicking in and rehydrating the machine with state of a machine with completely different config. This probably could work most of the time (assuming your new config wouldn't be vastly different) but it could also lead to really obscure bugs and wasted developer time because they would act on the incorrect assumptions. I believe that it should be possible to bail out of the Fast Refresh and just trigger the full reload (for a component or the site) for times when we know that the new config is incompatible with the old one, but I have no idea how actually Fast Refresh is currently implemented and how we could potentially integrate with it.

@chreck
Copy link

chreck commented Jun 17, 2020

I am also using react-native and storing state and context. This is my current version, with some ideas from this issue. Maybe someone sees some optimisations or mistakes.

hooks.ts

// ideas from https://github.com/davidkpiano/xstate/blob/master/packages/xstate-react/src/useMachine.ts
// https://github.com/davidkpiano/xstate/issues/995#issuecomment-586497453

import { useEffect, useMemo } from 'react';
import {
    EventObject,
    interpret,
    StateMachine,
    Interpreter,
    AnyEventObject,
    InterpreterOptions,
    MachineOptions,
    Typestate,
    StateSchema,
    StateValue
} from 'xstate';

export interface IStateContext<
    TContext
> {
    context?: Partial<TContext>;
    stateValue?: StateValue;
}

export interface IUseMachineOptions<
    TContext,
    TEvent extends EventObject = AnyEventObject,
    TTypestate extends Typestate<TContext> = any,
    TStateSchema extends StateSchema = any
> {
    /**
     * The method is called when the interpreter is instantiated and can be started.
     * Hint: You need to call service.start() by yourself.
     */
    doStart?: (service: Interpreter<TContext, TStateSchema, TEvent, TTypestate>) => void;
    /**
     * The method is called when the instance is going to be destroyed.
     * Hint: You need to call service.stop() by yourself.
     */
    doStop?: (service: Interpreter<TContext, TStateSchema, TEvent, TTypestate>) => any;
}

export type ReactContextMachine<
    TContext,
    TEvent extends EventObject = AnyEventObject,
    TTypestate extends Typestate<TContext> = any,
    TStateSchema extends StateSchema = any
> = Interpreter<TContext, TStateSchema, TEvent, TTypestate>;

export const useMachine = <
    TContext,
    TEvent extends EventObject = AnyEventObject,
    TTypestate extends Typestate<TContext> = any,
    TStateSchema extends StateSchema = any
>(
    machine: StateMachine<TContext, TStateSchema, TEvent, TTypestate>,
    options: Partial<InterpreterOptions> &
        Partial<IUseMachineOptions<TContext, TEvent, TTypestate, TStateSchema>> &
        Partial<MachineOptions<TContext, TEvent>> = {}
): Interpreter<TContext, TStateSchema, TEvent, TTypestate> => {
    const {
        context,
        guards,
        actions,
        activities,
        services,
        delays,
        stateValue,
        doStart,
        doStop,
        ...interpreterOptions
    } = options;

    const machineConfig = {
        context,
        guards,
        actions,
        activities,
        services,
        delays
    };

    // tslint:disable-next-line: no-object-literal-type-assertion
    const createdMachine = machine.withConfig(machineConfig, {
        ...machine.context,
        ...context
    } as TContext);

    const service = useMemo(() => {
        const s = interpret(createdMachine, interpreterOptions);
        if (doStart) {
            doStart(s);
        } else {
            s.start();
        }
        return s;
    }, []);

    useEffect(() => {
        return () => {
            if (doStop) {
                return doStop(service);
            } else {
                return service.stop();
            }
        };
    }, []);

    return service;
};

Contexts.tsx

import React from 'react';
import AsyncStorage from '@react-native-community/async-storage';
import { StateValue, State } from 'xstate';

import { ReactContextMachine, IUseMachineOptions, IStateContext, useMachine } from './statemachine/hooks';

type MachineContext = ReactContextMachine<IContext, Events>;
// ... add here more context types

// tslint:disable-next-line: no-object-literal-type-assertion
export const MachineContext = React.createContext<MachineContext>({} as MachineContext);
// ... add here more contexts

export const Contexts = (props: any) => {

    const store = (location: string, context?: any, stateValue?: StateValue) => {
        return AsyncStorage.setItem(
            location,
            JSON.stringify({
                context,
                stateValue,
            })
        );
    };

    const load = async (location: string): Promise<IStateContext<IContext>> => {
        const raw = (await AsyncStorage.getItem(location)) || '{}';
        const data = JSON.parse(raw);
        const context = data.context || undefined;
        const stateValue = data.stateValue || undefined;
        return {
            context,
            stateValue
        }
    };

    const storeLocation = 'mylocation/tostore/mymachine/state/and/context';
    const options: IUseMachineOptions<IContext, Events, any, IStateSchema> = {
        doStart: async (service) => {
            try {
                const data = await load(storeLocation);
                if(data.stateValue) {
                    const initialState = State.from(data.stateValue, data.context);
                    service.start(initialState as State<IContext, Events, IStateSchema>);
                } else {
                    service.start();
                }
            } catch(e) {
                service.start();
            }
        },
        doStop: async (service) => {
            const stateValue = service.state.value;
            const context = service.state.context;
            await store(storeLocation, context, stateValue);
            return service.stop();
        },
    };
    const mymachine1 = useMachine<IContext, Events, any, IStateSchema>(stateMachine, options);
    // create here another machine
    return (
        <MachineContext.Provider value={mymachine1}>
            <MachineContext2.Provider value={mymachine2}>
                {props.children}
            </MachineContext2.Provider>
        </MachineContext.Provider>
    );
}

App.tsx

    public render() {
        const { isLoaded, initialRoute } = this.state;
        return isLoaded ? (
            <Contexts>
                <MyFirstScreenComponent />
            </Contexts>
        ) : <SplashScreen />;
    }

@raarts
Copy link

raarts commented Jun 21, 2020

So there is no solution to this yet? Can anybody explain what is the problem with @Nohac 's solution Hot-patching the library and replacing useConstant with useState seems to have solved the problem. ?

(I presume he was patching 1.0.0-rc.3?)

@gaearon
Copy link

gaearon commented Oct 15, 2020

Hey folks, sorry I just saw this thread.

We have documented Fast Refresh constraints here and there's another detailed explanation here. I hope those are helpful. I understand the frustration of having to adjust your code to work within these constraints, but it makes your library more resilient overall, and creates a lot of benefit in the long run.

If you have any specific questions about how to handle a particular scenario, feel free to file an issue in the React repo and I'd be happy to discuss and brainstorm.

@Andarist
Copy link
Member

@gaearon I'll be filing the issue in the following days. Thanks for the brainstorming offer 👍

@davidkpiano
Copy link
Member

I'm having the same problem in next.js now they've started using fast-refresh. Here's a tiny example of it breaking using next: idlefingers/next-xstate-fast-refresh-bug

Hope that helps!

I've confirmed that the fast refresh bug in this repo no longer occurs with the latest version of @xstate/react (version 1+).

@osequi
Copy link

osequi commented Dec 5, 2020

Hey 👍

I'm using the latest xstate, xstate/react and nextjs. I have the same problem. When navigating with browser's back / forward buttons all works fine. When using a next/link the state machine gets confused.

Please find the repo at: https://github.com/osequi/test-xstate/releases/tag/0.1.0

@osequi
Copy link

osequi commented Dec 5, 2020

A note: this might not be an XState issue. The same strange behavior was present even without XState.

@davidkpiano
Copy link
Member

@osequi Can you open a new issue with this? This doesn't sound like a Fast Refresh issue (I could be wrong, though).

@milanbgd011
Copy link

Possible solution for this problem: #2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests