-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improve comparison by using internal typescript methods #433
Conversation
…unction, function parameteres and classes
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.
big fixture file extracted from a grafana/ui PR to test an edge case
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.
big fixture file extracted from a grafana/ui PR to test an edge case
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.
Great enhancements 🙌
Left one question regarding the positional arguments.
initial clusterview plugin cleaned up for public release. compatibility check failing due to bug. Expected to be fixed shortly grafana/levitate#433
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.
🚢🇮🇹
What this PR does / why we need it:
This PR is similar to #361 but it doesn't put all the trust in the typescript internal checker.
In the previous PR, the internal ts checker would report errors between two identical packages because of internal ts nuances that created fake incompatible types.
The newer approach is to trust the typescript internal methods if it says two types are compatible but, if it says they are not, we always fallback to text comparison.
Additionally I added a test case to track an edge case with typescript autogenerated types suffixes.
Which issue(s) this PR fixes:
It closes #380
Fixes #
Special notes for your reviewer: