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

Warning severity configuration #3634

Closed
wants to merge 1 commit into from

Conversation

emereum
Copy link
Contributor

@emereum emereum commented Feb 24, 2023

This PR adds a new warningSeverity configuration option. This option allows consumers to promote specific warnings to exceptions. This solves the issue where some consumers have unique needs for error reporting, such as #3618, while still respecting MobX's error reporting strategy in the default case.

Example usage:

configure({
    warningSeverity: {
        computedRequiresReaction: "throw",
        enforceActionsStrict: "throw",
        enforceActionsNonStrict: "throw",
        observableRequiresReaction: "throw",
        derivationWithoutDependencies: "throw"
    }
})

Warnings will default to warn which is in line with their current behaviour in 6.8.0.

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2023

🦋 Changeset detected

Latest commit: cdc6ede

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mobx Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@emereum emereum marked this pull request as ready for review February 24, 2023 05:29
@urugator urugator removed their request for review February 25, 2023 10:03
@emereum
Copy link
Contributor Author

emereum commented Mar 27, 2023

Hey @iChenLei do you think there is a scenario where this change could be introduced? On one hand, it solves #3618, on the other I see how it might make triaging user bug reports more difficult - folks might promote warnings to errors, then submit bug reports here that aren't applicable to a default warning configuration.

In this case, we could suffix exceptions with something like: "You are seeing this error because you have configured MobX to throw when this occurs. Please revert your warning severity configuration and retry before reporting issues to MobX"

@emereum emereum closed this Apr 4, 2023
@emereum
Copy link
Contributor Author

emereum commented May 15, 2023

@urugator keen to get your thoughts on this PR as well as my comments above on the potential downsides and ways to mitigate them.

@emereum emereum reopened this May 15, 2023
@emereum emereum closed this Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant