Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

♻️ have withTiming return early if performance.timing does not ex… #1119

Merged
merged 1 commit into from
Oct 16, 2019

Conversation

michenly
Copy link
Contributor

@michenly michenly commented Oct 15, 2019

…ist.

Description

The performance mock currently return an empty object which mean calling new Performance will run into error during test.

I feel like I am not getting to the root of the issue with this solution.

Musing 1:

I think the performance mock come with jsdom, which only mock a portion of the API in performance but this does not explain why the mock is an empty object.

Musing 2:

we are using performance.timing, but checking for PerformanceNavigationTiming before use. Should we check for PerformanceTiming as well?

Musing 3:

PerformanceTiming is deprecated 😂, should the code be updated to use performance.navigation instead? Although the official types have timing and navigation type listed as deprecated

Type of change

  • @Shopify/react-performance - Major
  • @Shopify/performance - Patch

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

@michenly michenly force-pushed the add-check-to-performance branch 2 times, most recently from b1f8423 to 1550544 Compare October 15, 2019 18:09
Copy link
Contributor

@marutypes marutypes left a comment

Choose a reason for hiding this comment

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

Seems legit to me

packages/performance/CHANGELOG.md Outdated Show resolved Hide resolved
@michenly michenly force-pushed the add-check-to-performance branch 5 times, most recently from 1440d7d to 7d8e512 Compare October 15, 2019 21:19
@michenly michenly self-assigned this Oct 15, 2019
@michenly
Copy link
Contributor Author

actually changed the way I solved this. It feels less hacky now.

@michenly michenly force-pushed the add-check-to-performance branch from 7d8e512 to 9989181 Compare October 16, 2019 15:25
…ist.

Co-Authored-By: Gord P <GoodForOneFare@users.noreply.github.com>
@michenly michenly force-pushed the add-check-to-performance branch from 9989181 to 78e273f Compare October 16, 2019 18:47
@michenly michenly merged commit 807204f into master Oct 16, 2019
@michenly michenly deleted the add-check-to-performance branch October 16, 2019 18:48
@michenly michenly temporarily deployed to production October 16, 2019 19:42 Inactive
@marutypes marutypes temporarily deployed to gem October 23, 2019 21:05 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants