search

mobxjs / mobx-state-tree

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

return undefined in resolveNodeByPathParts when failIfResolveFails = false #2071

Closed BrianHung closed 9 months ago

BrianHung commented 10 months ago

This PR fixes a bug in resolveNodeByPathParts where when getChildNode throws a fail and failIfResolveFails=false, undefined was not returned. Instead, resolveNodeByPathParts let that error propagate through.

Each of these complex types has the ability to fail and throw an error.

https://github.com/mobxjs/mobx-state-tree/blob/01d0fff5ba5f72a0b517582099ea3be05b4e1936/packages/mobx-state-tree/src/types/complex-types/map.ts#L326

https://github.com/mobxjs/mobx-state-tree/blob/01d0fff5ba5f72a0b517582099ea3be05b4e1936/packages/mobx-state-tree/src/types/complex-types/array.ts#L161

https://github.com/mobxjs/mobx-state-tree/blob/01d0fff5ba5f72a0b517582099ea3be05b4e1936/packages/mobx-state-tree/src/types/complex-types/model.ts#L637

coolsoftwaretyler commented 9 months ago

Hey @BrianHung - sorry this has sat for so long, been the busy season for me, but I have a little more time over the coming weeks and with 5.2.0 released today, I'd love to close out some PRs and get us ready for some new releases.

When you get a chance, will you please update the description here and talk about the why of this PR? From there, we'll also need some tests and documentation updates (unless this is fixing a bug where we already have test coverage/documentation for the behavior).

Thanks!

chakrihacker commented 9 months ago

@BrianHung Any update??

BrianHung commented 9 months ago

@chakrihacker Just updated the PR description. Will try to write a test. I have added a test.

coolsoftwaretyler commented 9 months ago

🍾 🥳 Thanks @BrianHung! I really appreciate it! Makes sense and LGTM. This'll make it into the next release. I'm probably going to release the next version after the repo restructure, so make sure those changes allow us to ship the way we have been for the most part.