mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.94k stars 641 forks source link

Middlewares can not detect modification directly made in the parent action #1948

Open weiwei-lin opened 2 years ago

weiwei-lin commented 2 years ago

Bug report

Description The documentation on addMiddleware says

Middleware can be used to intercept any action is invoked on the subtree where it is attached. If a tree is protected (by default), this means that any mutation of the tree will pass through your middleware.

However, this is not true. When the parent action modify the child directly, the middleware cannot detect the action. This either needs to be fixed. Or at least the function documentation needs to be updated to reflect the actual behavior.

Sandbox link or minimal reproduction code sandbox

Describe the expected behavior Click on "modify from parent". Console should print "action detected"

Describe the observed behavior Click on "modify from parent". Console did not print "action detected"

coolsoftwaretyler commented 1 year ago

@weiwei-lin - thanks for the Sandbox, and apologies for taking so long to get back to you. I see what you mean. I think this is a case of the documentation being unclear/conflicting.

The documentation you posted here conflicts with (my interpretation of) what we say in the middleware concepts doc about targets:

the middleware will only be attached to actions of the target and further sub nodes of such.

The behavior in your Sandbox is consistent with the conceptual documentation, but not the actual documentation for addMiddleware.

@jamonholmgren - do you have thoughts/knowledge on which interpretation is the intended behavior?

zuhorer commented 1 year ago

issue moved to https://github.com/coolsoftwaretyler/mst-middlewares/issues/5

coolsoftwaretyler commented 1 year ago

^ Whoops, we are not moving this over there. This got captured in a big issue migration because it looked like a middlewares issue, but I think this is just a docs problem. Leaving here and open for now.