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

Include only necessary fonts for target environment specified by Browserslist #1674

Merged
merged 12 commits into from
Dec 2, 2018
Merged

Conversation

ylemkimon
Copy link
Member

@ylemkimon ylemkimon commented Aug 28, 2018

Fixes #1667.
Closes #1495.

+ Allow WOFF and WOFF2 to be controlled using environment variables like TTF. (USE_TTF) These environment variables will override target environment.

Uses caniuse data to detect support. No additional dependencies are required, as they are already included in other dependencies.

…serslist

Allow WOFF and WOFF2 to be controlled using environment variables
@codecov
Copy link

codecov bot commented Aug 31, 2018

Codecov Report

Merging #1674 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1674   +/-   ##
=======================================
  Coverage   93.67%   93.67%           
=======================================
  Files          80       80           
  Lines        4666     4666           
  Branches      813      813           
=======================================
  Hits         4371     4371           
  Misses        261      261           
  Partials       34       34
Flag Coverage Δ
#screenshotter 88.62% <ø> (ø) ⬆️
#test 85.04% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cda184b...2924351. Read the comment docs.

@ylemkimon ylemkimon added blocked and removed blocked labels Sep 3, 2018
@ylemkimon ylemkimon removed this from the Version 0.10.0 milestone Sep 8, 2018
@kevinbarabash kevinbarabash self-assigned this Nov 3, 2018
Copy link
Member

@kevinbarabash kevinbarabash left a comment

Choose a reason for hiding this comment

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

I tried BROWSERSLIST="Chrome 68" USE_WOFF2=true yarn build and the fonts worked. The dist/fonts only contained .woff2 files. I checked dist/katex.js and the code had been compiled down to ES5. I tried BROWSERSLIST="Chrome 68" yarn build with the same results.

@ylemkimon
Copy link
Member Author

@kevinbarabash There was a bug in #1595, where Browserslist config is not read even if isESMBuild is false. Also, I've upgraded uglifyjs-webpack-plugin to terser-webpack-plugin, to be able to minify ES6 output.

@codecov-io
Copy link

Codecov Report

Merging #1674 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1674   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files          80       80           
  Lines        4653     4653           
  Branches      807      807           
=======================================
  Hits         4358     4358           
  Misses        261      261           
  Partials       34       34
Flag Coverage Δ
#screenshotter 88.85% <ø> (ø) ⬆️
#test 84.99% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64745b5...e388fdd. Read the comment docs.

Copy link
Member

@kevinbarabash kevinbarabash left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I finally got around to testing this again. The browserslist setting is working correctly. Thanks for adding this feature. I'm personally looking forward to using this at Khan Academy once we finish our migration to webpack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants