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.

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

Use browserslist to configure which font formats to supply subsets and fallbacks in, and whether to add the JS-based preload polyfill #120

Merged
merged 8 commits into from
Aug 1, 2020

Conversation

papandreou
Copy link
Collaborator

@papandreou papandreou commented Jul 26, 2020

Fixes #119

The current set of default browsers happens to result in woff+woff2 and the JS preload polyfill being selected atm., which is luckily the same as our current --formats default 🎉

TODO:

  • Also wire it up to control whether the JavaScript-based preload polyfill is included.

@papandreou papandreou self-assigned this Jul 26, 2020
@coveralls
Copy link

coveralls commented Jul 26, 2020

Pull Request Test Coverage Report for Build 535

  • 28 of 28 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 90.196%

Totals Coverage Status
Change from base Build 524: 0.02%
Covered Lines: 1145
Relevant Lines: 1219

💛 - Coveralls

@papandreou papandreou force-pushed the feature/browserslist branch from ed45a7d to 0e1e6fb Compare July 26, 2020 20:59
@papandreou papandreou changed the title Use browserslist to configure which font formats to supply subsets and fallbacks in Use browserslist to configure which font formats to supply subsets and fallbacks in, and whether to add the JS-based preload polyfill Jul 26, 2020
@papandreou papandreou marked this pull request as ready for review July 26, 2020 21:00
@papandreou papandreou force-pushed the feature/browserslist branch from 0e1e6fb to 2bf7cf4 Compare July 26, 2020 21:05
@papandreou papandreou requested a review from Munter July 26, 2020 21:09
Copy link
Owner

@Munter Munter left a comment

Choose a reason for hiding this comment

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

I thought the implementation would require much more complexity :)

I only have reservations about the CLI interface

lib/parseCommandLineOptions.js Outdated Show resolved Hide resolved
type: 'array',
choices: ['woff2', 'woff', 'truetype'],
})
.options('js-preload', {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure we should expose this option. I'd rather promote usage of browserslist project configuration to have the tool just do the right thing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, yeah, I agree. I've taken it out in dccddcf

formats = ['woff2'];
if (
_.intersection(
browsersList('supports woff, not supports woff2'),
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't seen this interface before. That's pretty awesome!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it's pretty nifty. A shame that we have to use lodash to combine it with the user's query though.

@papandreou
Copy link
Collaborator Author

@Munter, good to go in?

@Munter
Copy link
Owner

Munter commented Aug 1, 2020

Yeah. Let's merge

@papandreou papandreou merged commit db7bb09 into master Aug 1, 2020
@Munter Munter deleted the feature/browserslist branch August 3, 2020 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support .browserslistrc to enable legacy output
3 participants