Skip to content

fix: load IBM Plex box-drawing glyphs - #1113

Open
dcavalcante wants to merge 1 commit into
nodejs:mainfrom
dcavalcante:fix/ibm-plex-box-drawing
Open

dcavalcante wants to merge 1 commit into
nodejs:mainfrom
dcavalcante:fix/ibm-plex-box-drawing

Conversation

@dcavalcante

@dcavalcante dcavalcante commented Sep 27, 2026 •

Copy link
Copy Markdown

Description

Add the official IBM Plex Mono Pi subset for Unicode box-drawing characters.

The existing IBM Plex Mono webfont is the Latin subset, which doesn't contain
unicodes U+2500–U+257F. On platforms where the preceding system monospace fonts are
unavailable (like Android), box-drawing characters can fall back to a different font with
incompatible metrics.

The Pi subset uses the same IBM Plex Mono family and is restricted with
unicode-range, so it is fetched only when box-drawing glyphs are required.
It is not added to the global font preload list.

The vendored WOFF2 is IBM's unmodified official Pi subset and its OFL license
is included alongside it.

Validation

  • node --run test
  • node --run format:check
  • node --run lint
  • verified the generated package includes the Pi WOFF2 and OFL license
  • generated Node.js Learn locally with the modified package
  • verified the box-drawing diagrams render correctly on Samsung and Motorola Android devices
  • verified the Pi subset is loaded on demand for box-drawing content

Visual before/after validation is included in nodejs/nodejs.org#9177.

Related Issues

Related to #909

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run node --run test and all tests passed.
  • I have check code formatting with node --run format:check & node --run lint.
  • I've covered new added functionality with unit tests if necessary.

@dcavalcante
dcavalcante requested a review from a team as a code owner September 27, 2026 23:14
@vercel

vercel Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
api-docs-tooling Ready Ready Preview Sep 27, 2026 11:31pm UTC

Request Review

@avivkeller avivkeller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this?

If so, we should import it from Fontsource, if it exits, rather than our own copy.

@dcavalcante

Copy link
Copy Markdown
Author

Do we need this?

If so, we should import it from Fontsource, if it exits, rather than our own copy.

Hey @avivkeller, yeah we need this on Android.. otherwise the boxes look very broken (checkout the screenshot in nodejs/nodejs.org#9177)

I checked @fontsource/ibm-plex-mono@5.3.0 and its published subsets are only cyrillic-ext, cyrillic, vietnamese, latin-ext, and latin... none covers U+2500–U+257F. IBM upstream does provide those glyphs in its Pi subset (which covers U+2500–U+259F). The WOFF2 in this PR is that unmodified IBM Pi subset.

So unfortunately there isn’t a Fontsource subset that can be imported for this. I kept the existing Fontsource dependency for Latin and added only the Pi subset needed for the box-drawing glyphs.

I saw that IBM also publishes @ibm/plex-mono, and that package does include the Pi subset. But switching to it would introduce a new dependency, and its package currently has a postinstall using @ibm/telemetry-js.

@MattIPv4

Copy link
Copy Markdown
Member

$0.02, I would be supportive of switching over to IBM's own authoritative package for the font if that contains everything we need, removing the fontsource dep completely?

Or, can we ask fontsource to include what we need upstream, and wait for that to be addressed?

@dcavalcante

Copy link
Copy Markdown
Author

$0.02, I would be supportive of switching over to IBM's own authoritative package for the font if that contains everything we need, removing the fontsource dep completely?

Or, can we ask fontsource to include what we need upstream, and wait for that to be addressed?

I was just gonna comment that I had opened a discussion to include the Pi set on Fontsource:
fontsource/fontsource#1387

IBM's own @ibm/plex-mono package does contain both the complete WOFF2 files and the split subsets, including IBMPlexMono-Regular-Pi.woff2, so switching away from Fontsource is technically possible... My only hesitation is that @ibm/plex-mono currently has a postinstall script through @ibm/telemetry-js, so I wasn't sure whether adding that dependency would be preferable here.

@MattIPv4

Copy link
Copy Markdown
Member

My only hesitation is that @ibm/plex-mono currently has a postinstall script through @ibm/telemetry-js, so I wasn't sure whether adding that dependency would be preferable here.

I haven't looked at the package, but I assume that postinstall isn't required? We use pnpm here, so we'd just not add it to our list of permitted postinstall's, though downstream consumers would have to make their decision themselves unless we bundle the dep.

Given you've opened that upstream issue though, I think I'd prefer to give that a few days to see if it goes anywhere (or ideally we can land the change upstream ourselves), before switching which package we consume?

@dcavalcante

Copy link
Copy Markdown
Author

Given you've opened that upstream issue though, I think I'd prefer to give that a few days to see if it goes anywhere (or ideally we can land the change upstream ourselves), before switching which package we consume?

Indeed.. postinstall is only for @ibm/telemetry-js.. it isn't required for the font assets themselves, and this repo can leave it unapproved through pnpm's allowBuilds.

But yeah, let's see how fast they respond to the issue.. If it stalls, I can draft the PR to try to move it along.

This branch was successfully deployed

1 active deployment
Preview – api-docs-tooling — dde8385b Deployed Sep 27, 2026 by vercel[bot]
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.

3 participants