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

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

Open
fmp777 opened this issue Feb 15, 2022 · 4 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@fmp777
Copy link

fmp777 commented Feb 15, 2022

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.

@fmp777 fmp777 added the bug Something isn't working label Feb 15, 2022
@henri-hulski
Copy link
Member

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
Copy link

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
Copy link

abirtley commented Jun 3, 2022

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants