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

[CP] Space character should be optional when tree shaking fonts #132883

Closed
eyebrowsoffire opened this issue Aug 19, 2023 · 3 comments
Closed

[CP] Space character should be optional when tree shaking fonts #132883

eyebrowsoffire opened this issue Aug 19, 2023 · 3 comments
Assignees
Labels
cp: approved Approved cherry-pick request cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. cp: review Cherry-picks in the review queue

Comments

@eyebrowsoffire
Copy link
Contributor

Issue Link

#132711

Commit Hash

6f227c0

Target

stable

PR Link

#132882

Changelog Description

Fixes an issue with tree shaking fonts that do not contain a space glyph.

Impacted Users

Flutter Web apps that have custom fonts that do not contain a space glyph

Impact Description

The flutter tool fails out with an error when attempting to compile an app that uses a custom icon font that does not contain a space.

Workaround

The user can pass --no-tree-shake-icons to prevent icon tree shaking, which will allow the flutter tool to complete the build process. However, it skips icon tree shaking, which bloats the payload size of their font files.

Risk

low

Test Coverage

yes

Validation Steps

Unit tests verify that the tool is feeding the proper into the font subset tool. Also, verified that the repository provided in #132711 can now build for web without issue.

@eyebrowsoffire eyebrowsoffire added the cp: review Cherry-picks in the review queue label Aug 19, 2023
@zanderso zanderso added the cp: approved Approved cherry-pick request label Aug 21, 2023
@zanderso
Copy link
Member

CP LGTM. The linked PR depends on the Engine CP flutter/engine#44869 landing first.

@itsjustkevin
Copy link
Contributor

Looks like flutter/engine#44869 is just waiting on a review. Will merge once it is reviewed as all tests are already passing.

@itsjustkevin itsjustkevin added cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. labels Aug 22, 2023
@github-actions
Copy link

github-actions bot commented Sep 7, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cp: approved Approved cherry-pick request cp: merge-to-stable Cherry-picks that should be merged to stable cp: merged Cherry-pick has been merged to the release branch. cp: review Cherry-picks in the review queue
Projects
None yet
Development

No branches or pull requests

4 participants