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

Globally tracking actions #2

Closed
antitoxic opened this issue Jul 25, 2016 · 7 comments
Closed

Globally tracking actions #2

antitoxic opened this issue Jul 25, 2016 · 7 comments

Comments

@antitoxic
Copy link
Collaborator

antitoxic commented Jul 25, 2016

Hi @zalmoxisus . @mweststrate mentioned the project.

I'm a fan :)

Just a few questions regarding handling mobx. In Mobx, stores are usually nested. I see the todo example in which remotedev() is called on each nested store instance. Do you think this is optimal?

In my opinion, a useful option forremotedev, one that covers most of the cases, will be to enable spying on all mobx observables or all nested ones at least without the need to wrap the nested stores.

Why? - because right now, I need to switch from a parent store to a child store to see a change which affects both. Correct me if I'm wrong here.

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jul 25, 2016

@antitoxic, thanks a lot for the feedback and the suggestion!

I'm going to cut 0.1.0 these days where it will be possible not only to log actions, but also to control the application: to toggle actions and dispatch actions remotely. It wouldn't be possible to do that in your scenario, as we need access for every class and to separate them in instances.

Though it's a good idea to just use remotedev() for logging only. We'll implement it after the current release.

Why? - because right now, I need to switch from a parent store to a child store to see a change which affects both. Correct me if I'm wrong here.

After we get zalmoxisus/redux-devtools-extension@6e23813 published, you will be able to open separate windows for every instance like so:

5lw3ypkrdd

@antitoxic
Copy link
Collaborator Author

antitoxic commented Jul 25, 2016

... it will be possible not to log actions, but also to control the application: to toggle actions and dispatch actions remotely ...

Actions are just functions in mobx, right? Would you keep reference to the function? I'm interested in how it will be done.

I am working locally on a version that supports nested observables by relying on the following info:

  1. Mobx is syncrounous. (I'm hoping this also applies for spy(). I asked @mweststrate and waiting for a reply). This means you don't have to do schedule(name, action) and you can do schedule(action) and control the name in the send(name).
  2. You can trigger tracking for all nested observables by accessing all of them in a observer. Accessing all of them is done while serializing so serialization is a good trigger.

Sample working code:

export default function spy(store, config) {
    init(store, config);
    if (isSpyEnabled) return;
    isSpyEnabled = true;
    reaction(() => serialize(store), function () {}); // trigger tracking of all observables
    mobx.spy((change) => {
        if (!change.spyReportStart) return;
        if (change.type === 'reaction') {
            try {
                var objName = getName(change.object.observing[0].sourceObject); // refer to the serialization reaction above
                send(objName)
            }
            catch (e) {
                return; // TODO: show other reactions
            }
        }
        if (change.type === 'action') {
            const action = createAction(change.name);
            if (change.arguments && change.arguments.length) action.arguments = change.arguments;
            schedule(action);
        } else if (change.type && mobx.isObservable(change.object)) {
            schedule(createAction(change.type, change));
        }
    });
}

I was going to give you a link to a fork but I saw the repo was updated since I started editing and I'm not in a mood for conflict resolution :)

What do you think about the code above?

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jul 25, 2016

Actions are just functions in mobx, right? Would you keep reference to the function? I'm interested in how it will be done.

Yes, I'd like to send all the functions name to the monitor and just select which function want to call from UI. But for now you just indicate the function name as type (usually the action name is the same, otherwise it will not be called) and arguments if necessary. The implementation is rather simple. You can give it a try ;)

I am working locally on a version that supports nested observables by relying on the following info... What do you think about the code above?

Looks interesting, but still wondering if mixing stores in one instance would be reasonable. We wouldn't be able even to show diffs, which is by default when opening the monitor. Autogenerating instances wouldn't work as well.

Will try your fork. Thanks for working on this!

I was going to give you a link to a fork but I saw the repo was updated since I started editing and I'm not in a mood for conflict resolution :)

I will change only monitorActions.js and utils.js' setValue from now, so spy.js is free for experiments :)

@antitoxic
Copy link
Collaborator Author

... We wouldn't be able even to show diffs, which is by default when you open the monitor

Actually diffs are possible. I got them working. I don't understand what you mean by "Autogenerating instances wouldn't work as well."

@zalmoxisus
Copy link
Owner

zalmoxisus commented Jul 25, 2016

I thought we'll have separate states for every object, so it wouldn't make sense to have a diff between { name: 'Todo 1', finnished: true } and { todos: [ { name: 'Todo 1', finnished: true } ] }. But if the state will be for the parent object including nested objects, it makes sense 👍

If this "hack" with serialization is viable and works for classes as well, it will be indeed a better solution than calling remotedev on each nested store (having hundreds of instances would make it hard to use).

Would you like to send a PR?

@antitoxic
Copy link
Collaborator Author

Working on it. 👍

@zalmoxisus
Copy link
Owner

Closing in favour of #5 and #6.

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

2 participants