Font library: lib-font as npm dependency #54440
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Tonya Mork <tonya.mork@automattic.com>
vcanales
added
[Type] Enhancement
This changed required applying a change in the webpack config suggested by lib-font's developer, detailed [here](https://github.com/Pomax/lib-font/#how-do-i-use-this-with-webpack). Closes #53653
vcanales
force-pushed
the
try/lib-font-as-npm-dep
branch
from
74602c2
to
f9f2f0d
Compare
There was a problem hiding this comment.
👋 Thanks for digging into this.
The webpack config added in this PR is the same we are using in Create Block Theme plugin webpack config.
In the original issue description I pointed out that @youknowriad suggested avoiding changing the Gutenberg's webpack config. That was the main reason to open the issue.
I'm not sure about all the outcomes around changing the webapck config, so It would be useful to get a review from Riad.
|
Size Change: -77.9 kB (-5%) ✅ Total Size: 1.56 MB
ℹ️ View Unchanged
|
|
It's just a shame that the package requires a special config of your bundler. I can live with that but remember that you'll have to update Core's webpack config as well. Fortunately, it's the edit-site package and not a generic package that is used often by third-party JS devs. |
I agree, but since we're not getting help with the library from the dev, this and forking it seem like the two only choices if we want to keep it. I wouldn't mind maintaining a fork; I don't know if there's precedent for incorporating this sort of library to the WordPress org as a fork. |
@youknowriad if you are okay with importing the package like this, at least for this next release, please approve the PR. |
matiasbenedetto
added
the
Backport from WordPress Core
matiasbenedetto
requested changes
Sep 14, 2023
There was a problem hiding this comment.
@vcanales please, remember that to make the library work as expected apart from adding the lib-font main file, the other 2 files (inflate and unbrotli) are required to uncompress the font files. In Create Block Theme plugin we are adding them like this:
|
👍 Please don't wait for my approval, this LGTM |
youknowriad
added
Backport to WP 6.7 Beta/RC
matiasbenedetto
requested review from
spacedmonkey,
kevin940726 and
Mamaduka
as code owners
3 tasks
mikachan
removed
the
Backport to WP 6.7 Beta/RC
|
@vcanales Is this PR still relevant? If so are you targeting for WP 6.5? |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change required applying a change in the webpack config suggested by lib-font's developer, detailed here.
Closes #53653