Xstate: spawnMachine method is ignoring custom classes inherited from Interpreter.

[Baael]

Description
If using custom interpreter class inherited from Interpreter, spawnMachine method always returns original class.

Expected Result
CustomInterpreter class should create new instance of itself in spawnMachine method.

Actual Result
Interpreter creates new instance of itself, not pointed class.

Reproduction

class CustomInterpreter extends Interpreter {
    spawnMachine(machine, options) {
        return super.spawnMachine(machine, options)
    }
}
const actor = new CustomInterpreter(exampleMachine)
assert( actor.spawnMachine(exampleMachine) instanceof CustomInterpreter ) // assert error

Additional context

At core/src/interpreter.ts line 848

Line:

const childService = new Interpreter(machine, {

IMHO should be something like:

const childService = new (this.constructor as any)(machine, {

I have prepared fix for it already, I want to be sure if it is bug or feature.

[Andarist]

Personally I would be against this - I feel that extending our classes can only lead to problems. Composition over inheritance and stuff. Why would you like to extend the builtin Interpreter like this?

[Baael]

According to documentation guides, there is a possibility to create custom interpreters, however custom interpreters do not work with nested machines, so only option is to extend class.
While creating multiple compositions of machines, sometimes they are used as plain sometimes as nested, I would expect same behavior in all cases.

Simplest example: because of duplication of code we decided to migrate promises and some services to machines. Thanks to that we can compose or use some machines standalone, also testing is universal.
While process of migration is ongoing and we needed to keep already existing error handling, instead of fighting with it in every case, we created function wrapper that returns machines as promises. Yet it was not handy, and we wanted to keep machine descriptions as much in JSON format as possible.

Extending Interpreter class allowed us to get things done and solve problems instead of fighting with them. In this case future problems are something we can handle and we agree for.

Simply thanks to that this part of code is not changing, and can be used as promise and machine.

load: {
  invoke: {
    src: loadService,
    onDone: 'done',
    onError: 'error',
  }
}

So few lines of code solved problem instead of researching for alternative libraries, writing our own, or refactoring every single machine. We are using a lot MST now, custom interpreter allowed us also to simplify stores and for painless and powerful integration of XState with MST. And it is working great!

Also solves problem from #696 that we were fighting with.

[Baael]

Why would you like to extend the builtin Interpreter like this?

Interpreter should be a class that interprets machine definition to custom resources, that allows two things: keeping things universal, and making easy to integrate Xstate with any possible system. Let's assume that having mobx-state-tree model, we want to add state machine on top of it, and we love how xstate is solving it, meanwhile there is a lot of unnecessary duplication of code and logic. So why do not create MSTXstateInterpreter, that will do something like this:

class MSTXstateInterpreter extends Interpreter {
    exec(action, context, event, actionFunctionMap) {
        const actionOrExec = getActionFunction( action.type, context ) 
        const exec = typeof actionOrExec === 'function' ? actionOrExec : actionOrExec ? actionOrExec.exec : action.exec;
        if (exec) {
            return exec(context, event, { action: action, state: this.state });
        } else {
            return super.exec(action, context, event, actionFunctionMap)
        }
    }
}

_obviously it is example not finished code_

instead of writing and mapping guards, services or actions to model. Possibilities are endless, and while we are interpreting xstate definition to our own resources, we can still keep it as it is, without breaking compatibility etc.

IMHO Interpreter class should be exposed, devs should be even encouraged to use it as it may ends up with deep integration of xstate in multiple different projects. And obviously, every developer that is using custom interpreter should know and be aware about consequences and take responsibility (like with any library).

[Acanthostega]

We are in this situation too. We need a way to register for our application the services that are created/spawned but since the default interpreter always spawns an instance of itself, we also have to do some hacks to work around this problem, or write too much boilerplate code.

I understand the problem of the composition but rewriting completely an interpreter just to change a particular behavior seems to be a big problem when we don't want to diverge from the default interpreter. Keeping composition can be also possible but we need a way to inject the interpreter to use when invoking a machine, but it will be necessary to provide this option in the config of the machine that should not be aware of such things for me.

What do you think of this problem? Is there another better solution?

@Baael do you still have your fix working? We can also make a PR if necessary.

[Andarist]

What do you think of this problem? Is there another better solution?

Could you describe your problem in a more concrete way? It's hard to discuss any details with such an abstract description. What are you trying to do?

[Baael]

@Baael do you still have your fix working? We can also make a PR if necessary.

Nope, in the end I've created my own implementation based on/compatible with Xstate build on MST, cause I've had multiple other things to solve that do not align well with Xstate itself.

[davidkpiano]

If you're creating your own interpreter, it is not recommended to extend Interpreter unless you know all the workarounds - instead, you should create your own standalone interpreter (as you have managed to do, which is awesome!).

Extending Interpreter is not a supported use-case.

[Andarist]

With that being said - you might be able to cover your use case more easily in v5 (in the works) with a custom spawn action, we decouple those from the core, so it should be easier to support other actor types, including custom interpreters.