-
Notifications
You must be signed in to change notification settings - Fork 578
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
Enable strictNullChecks for integration tests #4193
Conversation
4bc48d8
to
43c60ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test should make assertions instead of throwing exceptions. Maybe this warrants a quick discussion.
e050c81
to
d7e583e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love for me or you (probably me) to do a follow-up PR making this strict
instead of simply strictNullChecks
.
If you have the time, that would be awesome, but if not I am happy to look at it once my current bit of work is out of the way (probably end of the week). I'll create an issue to track |
What, How & Why?
This change is required for flexible sync. Ideally we'd enable
strict: true
but this throws a lot more errors so just doingstrictNullChecks
for now.I've tried to do runtime assertions where I thought it might make sense (e.g. there's a chance the thing might not exist, if something is badly broken, so throw) and otherwise use non-null assertions, but happy to change if people think we should take a different approach.
☑️ ToDos
Compatibility
label is updated or copied from previous entryBreaking
label has been applied or is not necessaryIf this PR adds or changes public API's: