Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Drop caniuse #2296

Merged
merged 3 commits into from Apr 29, 2019
Merged

Drop caniuse #2296

merged 3 commits into from Apr 29, 2019

Conversation

antross
Copy link
Member

@antross antross commented Apr 22, 2019 •
edited

Pull request checklist

Make sure you:

For non-trivial changes, please make sure you also:

  • Added/Updated related documentation.
  • Added/Updated related tests.

Short description of the change(s)

Includes updating all hints which previously used caniuse-api to use the new isSupported helper based on MDN data. Also cuts hint-compat-api over to using the packaged MDN data from @hint/utils shaving ~30KB off the bundle size (bringing it to ~618KB).

I'll leave migrating hint-compat-api to the new getUnsupported helper for a follow-up PR (which should help simplify that hint significantly).

@molant molant requested a review from sarvaje April 22, 2019 23:19
@antross antross force-pushed the drop-caniuse branch 2 times, most recently from 5cc60fb to a818a45 Compare April 24, 2019 20:39
@antross antross marked this pull request as ready for review April 24, 2019 20:39
@antross antross requested a review from molant as a code owner April 24, 2019 20:39
@antross antross force-pushed the drop-caniuse branch 2 times, most recently from aab9acf to 11999f4 Compare April 25, 2019 23:30
molant
molant approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@molant molant left a comment

Choose a reason for hiding this comment

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

Looks good. A couple comments maybe we should talk about early next week? Going to live it open so we don't forget about them but we should merge this early Monday (feel free to do it yourself).

Also I'm thinking we should link to the video of the PR review in the commit message as a Ref (https://youtu.be/Y94fMxmbMFI). It helped me a lot. What do you think?

packages/utils/src/compat/cache.ts Show resolved Hide resolved
packages/utils/src/compat/css.ts Show resolved Hide resolved
@molant
Copy link
Member

molant commented Apr 29, 2019

@antross there are conflicts with a package.json. Can you please fix it?

@antross
Copy link
Member Author

antross commented Apr 29, 2019

Yep, I'll get this updated shortly.

@antross antross mentioned this pull request Apr 29, 2019
Open
@antross
Copy link
Member Author

antross commented Apr 29, 2019

Also I'm thinking we should link to the video of the PR review in the commit message as a Ref (https://youtu.be/Y94fMxmbMFI). It helped me a lot. What do you think?

Yep, I like that - I'll add it.

@antross antross merged commit f934c5a into webhintio:master Apr 29, 2019
antross added a commit that referenced this pull request Apr 29, 2019
@antross
Breaking: Replace caniuse util with a compat util based on MDN data
74f0b67 
Close #2296
Fix #2097
Ref #2114
Ref https://youtu.be/Y94fMxmbMFI
@antross antross mentioned this pull request May 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@molant molant molant approved these changes

@sarvaje sarvaje Awaiting requested review from sarvaje

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@antross @molant