-
Notifications
You must be signed in to change notification settings - Fork 643
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
feat: add hasEnv, throw when getEnv finds no env #2039
Conversation
@jamonholmgren - when you get a chance, will you please take a look at this PR to add some error-throwing behavior to 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! |
48559d4
to
216991a
Compare
This will get released in the next breaking release. |
We're probably going to do a breaking release soon, so I will prepare this MR to merge soon! Thanks for bearing with us! |
Ok, one more attempt! Closing in favor of #2163 |
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:
yarn build-docs
, so there are a lot more documentation changes in the diff.packages/mobx-state-tree/__tests__/core/env.test.ts
where I thought it might be relevant to check howhasEnv
andgetEnv
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 throw error when env not found #938.packages/mst-middlewares/src/undo-manager.ts
than the original author did, since that middleware has changed over time.