Skip to content

Provided better errors when components are mis-configured #549

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

Merged
merged 3 commits into from
Apr 19, 2021

Conversation

gregtatum
Copy link
Member

Each commit here is well-ordered and can be considered separately.

Resolves #519.

The second commit isn't particularly risky, but the first introduces a new throw for misconfigured systems. See the commit message for more information.

dminor: As we discussed in Slack it would be great if you could look over the code. It's using TypeScript.

zibi: I also added you on here as a reviewer, as Dan wanted someone with JS experience to also look over it. Feel free to redirect if you know someone better to look at it.

The current behavior is permissive, and works without warnings. This
change makes it so that omitting the LocalizationProvider throws an
error. I was originally trying to decide between throwing an error and
emitting a console error. I opted for an error to have the behavior in
line with what the react-redux Provider does.

This is potentially a breaking change for users that are relying on the
LocalizationProvider being omitted before the bundles are loaded. This
behavior can still be achieved by providing an empty bundle list to the
LocalizationProvider.
@gregtatum gregtatum requested review from dminor and zbraniecki April 12, 2021 20:39
Copy link
Member

@dminor dminor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise: I think this will make debugging failures much easier! Thank you for adding the additional test cases and improving the names of the existing ones.

I don't see any issues here, beyond what the linter flagged. LGTM once those are fixed.

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.

In fluent-react, when an id is not found, it should report an error
3 participants