-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(overlay): validate that ConnectedPositionStrategy positions are passed in correctly at runtime #9466
Conversation
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 this will have to be a major version change since it's possible for an app to have an invalid position that by coincidence never gets used
throw Error('ConnectedPositionStrategy: At least one position is required.'); | ||
} | ||
|
||
this._preferredPositions.forEach(pair => { |
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.
Add a TODO to remove this check once Angular's template type checker is good enough to not need it?
…assed in correctly at runtime Since we have some cases where we pass in positions verbatim to the connected position strategy (the connected position directive), we could end up in a situation where the consumer passed in a set of invalid positions that don't break necessarily, but don't work correctly either. These changes add some sanity checks at runtime to make debugging easier.
11f154d
to
e6a5dd1
Compare
Addressed the feedback @jelbourn. |
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.
LGTM, will hold off merging until we switch master over to being 6.0
…assed in correctly at runtime (#9466) Since we have some cases where we pass in positions verbatim to the connected position strategy (the connected position directive), we could end up in a situation where the consumer passed in a set of invalid positions that don't break necessarily, but don't work correctly either. These changes add some sanity checks at runtime to make debugging easier.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Since we have some cases where we pass in positions verbatim to the connected position strategy (the connected position directive), we could end up in a situation where the consumer passed in a set of invalid positions that don't break necessarily, but don't work correctly either. These changes add some sanity checks at runtime to make debugging easier.