deriving fsm

[astanet]

Thanks for publishing this great module.

I want to make a derived store from fsm.

	const simpleSwitch = fsm('off', {
		off: { toggle: 'on' },
		on: { toggle: 'off' }
	});

	$: isOnReactive = $simpleSwitch === 'on' // this works
	const isOnDerived = derived(simpleSwitch, $simpleSwitch => $simpleSwitch === 'on') // this doesn't

Please take a look the following repl.
https://svelte.dev/repl/5448ab6468704700ae1c2db736a0ac44?version=3.48.0

Accessing derived store from fsm with $ causes this error.
Cannot read properties of undefined (reading 'unsubscribe')

Please let me know if I am wrong in any way.
Thanks.

[morungos]

You're not wrong. I've just today encountered this, too, and for me, it's a showstopper.

The good news is that I've dug in and I believe I may have found the issue. I still don't know what to do about it, but I know where it is.

The problem is that svelte-fsm returns a proxied object, so that functions can be automatically mapped to actions. Sadly for us, this wraps the subscribe function that's used to listen. The magic trick is that if the function found has a single argument, it's assumed to be a subscribe callback, and is passed to subscribe. If not... (i.e., if it has >1 argument) -- even if we are calling subscribe() -- it's called as an action function and we never get a value back. We therefore never get an unsubscribe function back (we get undefined), and the derived store crashes out on us with the message you found.

Svelte stores are no longer quite so simple as to work with a single callback argument, so the derived store is misinterpreted as requesting an action rather than a subscribe, therefore, no derived store can correctly subscribe. I'm surprised this even works on the rest of Svelte, to be honest.

I am not a huge fan of proxying anyway -- I'd probably rather have an explicit action invocation method (say, an invoke method), but its the subscribe / invoke confusion that's the problem.

It's even mentioned in the code: https://github.com/kenkunz/svelte-fsm/blob/96631f9da3ae510ab75341776305fee619fcdfa2/index.js, lines 59 to 73

"subscribe also behaves as an event invoker when called with any args other than a single callback" -- the problem is that Svelte itself will generate subscribes like this, even if you don't. In fact, there's sometimes a second invalidator argument to subscribe (which is not really documented, except it's there in the TypeScript files) and it's this that causes all the trouble, because it's essential for derived stores.

      /*
       * Proxy-based event invocation API:
       * - return a proxy object with single native subscribe method
       * - all other properties act as dynamic event invocation methods
       * - event invokers also respond to .debounce(wait, ...args) (see above)
       * - subscribe() also behaves as an event invoker when called with any args other than a
       *   single callback (or when debounced)
       */
      function subscribeOrInvoke(...args) {
        if (args.length === 1 && args[0] instanceof Function) {
          return subscribe(args[0]);
        } else {
          invoke('subscribe', ...args);
        }
      }

One solution would be to stop using number of arguments to detect a "real" subscribe, which would mean that "subscribe" becomes more or less a reserved action as it couldn't be invoked the same way. Although personally I don't think this overloaded subscribe trick is worth allowing anyway.

Which is a roundabout way of saying that for this module, subscribe doesn't properly return an unsubscribe function to a derived store.

[astanet]

@morungos Thank you for your detailed research and great insights. I could not have understood it in that depth. I am glad that I was not wrong, but rather that I was able to clarify this issue. I don't have the ability to provide a fix, but I hope this module will be even better and further accelerate development at Svelte.

[kenkunz]

@astanet, @morungos – thanks for reporting this and for the detailed analysis. Sorry for not commenting sooner. I'll take a look at #9 and try to address this w/in the next few days.

[morungos]

@kenkunz I have no idea why the second commit is it into the PR. I need it for my app and pushed it, but it's completely unrelated to this issue. I'll switch that to a branch for my private usage and remove from this PR. But it may take me a few hours to get to that as I'm away from dev tools right now.

The only commit to look at should be: 881e9ec

That second commit might be of interest, but it relates to automatic event less transitions, which is a much bigger issue.

[kenkunz]

Thanks for clarifying, @morungos.

[morungos]

OK, updated the PR now. Should be easier to review. Sorry it took so long to get to this.

[morungos]

I'm just following up on my to-do list. Turns out, that second commit I removed earlier, which was about adding eventless transitions, has been incredibly useful (aka, more or less essential) to our project, and they weren't hard to add in. If you're interested, check out: morungos@0955042 I'd be more than happy to make that a pull request too (we don't really want to maintain a fork), but if not, it can stay independently where it is.

[pragmat1c]

Any chance that PR #9 will get merged soon?

This library has been great so far (thanks @kenkunz !), but before I get too invested in it this looks something that would need to work.

[kenkunz]

Apologies for not merging #9 sooner – I had intended to do this a while ago, but was busy with a work project and it fell off my radar. Just merged it and published 1.2.0 to NPM. Thank you @astanet for reporting and @morungos for the PR!