cerebral / overmind

Overmind - Frictionless state management
https://overmindjs.org
MIT License
1.58k stars 95 forks source link

[BUG] svelte reactivity broken on subsequent state changes cross-components #546

Open fmp777 opened 2 years ago

fmp777 commented 2 years ago

REPL https://svelte.dev/repl/b911503421ab405e9a53c8455eaabe14?version=3.46.4

This REPL demonstrates how Overmind state does what is expected on FIRST click, but then subsequent clicks fails to react changes to the DOM entirely (cross component).

Next to it is the same thing with svelte-store - no problems.

henri-hulski commented 2 years ago

I think @mfeitoza has implemented overmind-swelte. So maybe he can help. He has also a version on his account maybe try it out. If it's ok we could merge it here.

https://github.com/mfeitoza/overmind-svelte

abirtley commented 2 years ago

I've been running into this too. The issue seems to arise when multiple components subscribe to the same state information. After the first state change, both subscriptions get called in (seemingly) the exact same manner, but the end result is that only one component remains subscribed to future state changes.

A (fairly lousy) workaround is to ensure that you only access the state at the top of your application, then pass that information down the component tree using component properties, rather than having components down the tree access the state directly. This may well be unacceptable for your use case.

I took a quick look at @mfeitoza 's fork, specifically the file https://github.com/mfeitoza/overmind-svelte/blob/master/src/overmindSvelte.ts

I edited my local node_modules files, changing node_modules/overmind-svelte/es/index.js:10 from

const tree = overmind.proxyStateTreeInstance.getTrackStateTree();

to

const tree = overmind.proxyStateTreeInstance.getTrackStateTreeWithProxifier();

and it solved the immediate problem. (I don't know if it's created any other issues yet, or whether the other changes in mfeitoza's fork are also desirable.)

I'll report back if I encounter any problems.

abirtley commented 2 years ago

Another issue with Overmind and Svelte is that, in production, the Svelte subscribers are not always called immediately when the state is updated.

Here is a REPL to demonstate: https://svelte.dev/repl/6461defeef214de7b2092cb427e40bf1?version=3.48.0

Everything works fine in dev mode, but if you build it in production mode, it will crash because the reactive assignments never get triggered (because their subscription listeners do not get invoked at the right time).

I'm wondering if the svelte-overmind mixin is too complex for its own good... The following is a super-simple overmind/svelte integration that seems to work fine in dev and production, and doesn't suffer from the cross-component issue described above.

type Context = IContext<typeof YourSvelteConfigObject>;
type ContextForSvelte = Context & {
  state: {
    subscribe: (subscription: (value: State) => void) => () => void;
    set?: (value: State) => void;
  };
};

const overmind = createOvermind(config);

// this mixin seems buggy, disabling it
// const storeMixin = createMixin(overmind) as ContextForSvelte;

/* eslint-disable fp/no-let, fp/no-mutation, fp/no-mutating-methods, fp/no-unused-expression */
let listeners: Array<(state: typeof overmind.state) => void> = [];
overmind.addMutationListener((/*mutation, paths, flushId*/) => {
  // console.log(`Notifying ${listeners.length} of state change`);
  listeners.forEach((l) => l(overmind.state));
});
const storeMixin: ContextForSvelte = {
  ...overmind,
  state: {
    ...overmind.state,
    subscribe: (listener: (state: typeof overmind.state) => void) => {
      // console.log('+++ Adding a listener');
      listener(overmind.state);
      listeners.push(listener);
      return () => {
        // console.log('--- Removing a listener');
        listeners = listeners.filter((l) => l !== listener);
      };
    },
  },
};
/* eslint-enable */
Ahmed-Ali commented 1 year ago

Just faced same issue. I love the approach Overmind trying to implement, but after seeing this has been open for more than a year, I don't think the Svelte implementation is mature enough to be taken seriously at the moment for anything other than pet projects.