fix(icons): generate static icons from resource SVGs (no font transforms)#8036
fix(icons): generate static icons from resource SVGs (no font transforms)#8036
Conversation
Generate changelog in
|
ggdouglas
force-pushed
the
gd/icons-static-components-from-resources
branch
from
b1601eb to
e9443e7
Compare
fix(icons): build static icon components from resource SVG pathsBuild artifact links for this commit: documentation | landing | table | demo | storybookThis is an automated comment from the deploy-preview CircleCI job. |
ggdouglas
force-pushed
the
gd/icons-static-components-from-resources
branch
from
e9443e7 to
16affa0
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Generates static @blueprintjs/icons React components directly from optimized resource SVG path data (matching core <Icon />), removing reliance on icon font glyph transforms to fix clipping/visual mismatches.
Changes:
- Switch icon component generation to use extracted
<path d="...">data fromresources/icons/for 16px/20px grids. - Add shared
extractPathsFromResourceSvg.mjsand reuse it in both path-module and component generation. - Escape/serialize generated path exports more safely via
JSON.stringify.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/icons/scripts/iconComponent.tsx.hbs | Renders multiple <path> elements from precomputed path arrays instead of transformed font glyphs. |
| packages/icons/scripts/generate-icon-paths.mjs | Reuses shared extraction helper and safely serializes exported path strings. |
| packages/icons/scripts/generate-icon-components.mjs | Generates components from resource SVG paths (16/20) and removes SVG font parsing. |
| packages/icons/scripts/extractPathsFromResourceSvg.mjs | New shared helper to optimize SVGs and extract d attributes from <path> elements. |
| packages/icons/package.json | Removes svg-parser devDependency no longer needed. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Tighten regex to match only path elements specificallyBuild artifact links for this commit: documentation | landing | table | demo | storybookThis is an automated comment from the deploy-preview CircleCI job. |
✅ Successfully generated changelog entry!Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. 📋Changelog Preview🐛 Fixes
|
Merge branch 'develop' into gd/icons-static-components-from-resourcesBuild artifact links for this commit: documentation | landing | table | demo | storybookThis is an automated comment from the deploy-preview CircleCI job. |
mm-wang
left a comment
mm-wang
left a comment
There was a problem hiding this comment.
Small suggestions, main thing I would prefer is some level of parallelization of the icon paths extraction. Open to discussing the error failing.
| * @param {string} iconName | ||
| * @returns {Promise<string[]>} | ||
| */ | ||
| export async function extractPathsFromResourceSvg(iconSize, iconName) { |
There was a problem hiding this comment.
thought: TODO comment on moving to a different file structure here? Just don't want to forget what we're moving toward.
There was a problem hiding this comment.
moving to a different file structure
Not 100% sure what you mean. Are you referring to switching these to TypeScript? Or something else?
| let paths16; | ||
| let paths20; | ||
| try { | ||
| paths16 = await extractPathsFromResourceSvg(16, iconName); |
There was a problem hiding this comment.
suggestion: Can we use a promise all for 16/20 to parallelize?
I'm also a bit unsure about throwing the errors at this point - it will break on the first icon being unable to extract, but won't necessarily catch all issues with the icons. Might be nice to gather them and note the errors later.
There was a problem hiding this comment.
Good idea! I’m going to keep this fail-fast for now; generation should halt on the first broken icon. f40b918
Parallelize path extraction in generatorBuild artifact links for this commit: documentation | landing | table | demo | storybookThis is an automated comment from the deploy-preview CircleCI job. |
Fixes #6329
Fixes #7558
Summary
@blueprintjs/iconscomponents from the same optimized paths asgenerate-icon-paths.mjs/<Icon />, instead of font glyphd+scale/translatetransforms.extractPathsFromResourceSvg.mjsshared by path modules and icon component generation; fix path extraction to used="..."capture (no stray quote ind).JSON.stringifyper path for safe escaping.Fixes visual mismatch / clipping issues discussed in #7558 / #6329; aligns with resource cleanup in #7826.
Before
After