-
Notifications
You must be signed in to change notification settings - Fork 108
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
Conditionally make calls to PerformanceObserver.takeRecords which doesnt exist in Safari #138
Conversation
…h doesnt exist in Safari
perfObservers[3].takeRecords(); | ||
if (typeof perfObservers[3].takeRecords === 'function') { | ||
perfObservers[3].takeRecords(); | ||
} |
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 assumed that the calls to logMetric
and poDisconnect
should still happen in this scenario, but let me know if that isn't the 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.
Interesting point, let me think out loud the logic.
Actually more I re-think the logic out loud more I think there is one more bug to solve 🤦🏻♂️
The final records are probably never saved in cls.value
.
@@ -46,5 +46,13 @@ describe('firstInput', () => { | |||
expect(spy).toHaveBeenCalledWith(2, 'cls'); | |||
expect(spy).toHaveBeenCalledWith(3, 'tbt'); | |||
}); | |||
|
|||
it('should conditionally call takeRecords', () => { |
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.
Thank you for adding the test
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.
🌲🚀🌕
@ernieturner just published v5.2.0 with your fix. Thank you again 🙏 |
Number of other Prettier changes in this changeset in files that I assume haven't been saved since the config was introduced.