search

mobxjs / mobx-state-tree

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

feat: add hasEnv, throw when getEnv finds no env #2039

Closed coolsoftwaretyler closed 6 months ago

coolsoftwaretyler commented 1 year ago

This PR should close #820, and is built off the initial work done by @t49tran in #938. I'm just opening this PR to re-implement on top of the master branch, rather than rebasing directly there.

There are a few differences between the diff on the initial PR and this one:

  1. I ran yarn build-docs, so there are a lot more documentation changes in the diff.
  2. I added a few additional expectations in packages/mobx-state-tree/__tests__/core/env.test.ts where I thought it might be relevant to check how hasEnv and getEnv work with these changes. There was also one test that had to be slightly modified that I don't think existed at the time of #938.
  3. I had to make a few more modifications to packages/mst-middlewares/src/undo-manager.ts than the original author did, since that middleware has changed over time.
coolsoftwaretyler commented 1 year ago

@jamonholmgren - when you get a chance, will you please take a look at this PR to add some error-throwing behavior to getEnv in MST? This should close #938, which you were looking for volunteers on a few years ago.

Based on our conversation about stability, do you think making this change still makes sense? Seems like a breaking change and would require a new major version.

I mostly wanted to get some small code-changing experience in the repo, and that old PR seemed like a good place to start. I found this work instructive, so even if we close both PRs, I won't be bummed about it or anything.

Thanks in advance!

jamonholmgren commented 1 year ago

This will get released in the next breaking release.

coolsoftwaretyler commented 7 months ago

We're probably going to do a breaking release soon, so I will prepare this MR to merge soon! Thanks for bearing with us!

coolsoftwaretyler commented 6 months ago

Ok, one more attempt! Closing in favor of https://github.com/mobxjs/mobx-state-tree/pull/2163