Skip to content

Commit 1951913

Browse files
committed
fix(icon): resolve per-collection @iconify-json packages
Resolution only understood the monolithic @iconify/json, which ships every collection in roughly 120MB. Projects overwhelmingly install the per-collection @iconify-json/<prefix> packages instead, and for those every <Icon> on every page rendered as an empty string: a nav of blank buttons with nothing in the markup to explain it. Both conventions now resolve, per-collection first. The project's own install is also checked ahead of require.resolve, which resolves relative to this module and would otherwise let a collection vendored inside stx shadow the one the app declared. A missing collection now warns once with the install command instead of silently rendering nothing.
1 parent d8d67ab commit 1951913

2 files changed

Lines changed: 149 additions & 11 deletions

File tree

packages/stx/src/builtins/icon.ts

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,62 @@ type IconCollection = Record<string, { body: string, width?: number, height?: nu
2323
// Cache loaded icon collections to avoid re-reading JSON files
2424
const collectionCache = new Map<string, IconCollection>()
2525

26+
/** Prefixes already reported as missing, so the warning fires once per run. */
27+
const warnedMissingCollections = new Set<string>()
28+
2629
/**
27-
* Resolve the on-disk path of an icon collection JSON, checking the
28-
* standard locations: the package resolver, then `node_modules/`, then
29-
* Stacks-style `pantry/` (which sits parallel to node_modules in apps
30-
* that use pantry as their vendor directory).
30+
* A missing collection used to render as an empty string, so every icon on the
31+
* page silently disappeared and the markup gave no hint why. Say it once, with
32+
* the install command, rather than leaving a blank UI to diagnose.
3133
*/
32-
function resolveCollectionPath(prefix: string): string | null {
33-
try {
34-
return require.resolve(`@iconify/json/json/${prefix}.json`)
35-
}
36-
catch { /* fall through */ }
34+
function warnMissingCollection(prefix: string): void {
35+
if (warnedMissingCollections.has(prefix)) return
36+
warnedMissingCollections.add(prefix)
37+
console.warn(
38+
`[stx] icon collection "${prefix}" is not installed, so its icons render as nothing. `
39+
+ `Install it with \`bun add -d @iconify-json/${prefix}\`.`,
40+
)
41+
}
3742

43+
/**
44+
* Resolve the on-disk path of an icon collection JSON.
45+
*
46+
* Two packaging conventions are accepted. `@iconify/json` ships every
47+
* collection in one ~120MB dependency; `@iconify-json/<prefix>` ships a single
48+
* collection in about a megabyte and is what most projects actually install.
49+
* Supporting only the monolith meant an app with `@iconify-json/lucide`
50+
* installed rendered nothing at all for every icon on every page, with no
51+
* error to explain it.
52+
*
53+
* Each convention is checked through the package resolver first, then against
54+
* `node_modules/` and Stacks-style `pantry/` (which sits parallel to
55+
* node_modules in apps that vendor there).
56+
*/
57+
export function resolveCollectionPath(prefix: string): string | null {
58+
// The consuming project's own install wins. `require.resolve` resolves
59+
// relative to THIS module, so it happily returns a collection vendored
60+
// inside stx's own dependencies even when the app never installed one —
61+
// which silently renders a different icon set than the app declared.
3862
const cwd = process.cwd()
3963
for (const candidate of [
64+
`${cwd}/node_modules/@iconify-json/${prefix}/icons.json`,
65+
`${cwd}/pantry/@iconify-json/${prefix}/icons.json`,
4066
`${cwd}/node_modules/@iconify/json/json/${prefix}.json`,
4167
`${cwd}/pantry/@iconify/json/json/${prefix}.json`,
4268
]) {
4369
if (existsSync(candidate)) return candidate
4470
}
71+
72+
for (const specifier of [
73+
`@iconify-json/${prefix}/icons.json`,
74+
`@iconify/json/json/${prefix}.json`,
75+
]) {
76+
try {
77+
return require.resolve(specifier)
78+
}
79+
catch { /* try the next convention */ }
80+
}
81+
4582
return null
4683
}
4784

@@ -56,7 +93,10 @@ function loadCollectionSync(prefix: string): IconCollection | null {
5693
if (collectionCache.has(prefix)) return collectionCache.get(prefix)!
5794

5895
const jsonPath = resolveCollectionPath(prefix)
59-
if (!jsonPath) return null
96+
if (!jsonPath) {
97+
warnMissingCollection(prefix)
98+
return null
99+
}
60100

61101
try {
62102
const data = JSON.parse(readFileSync(jsonPath, 'utf8'))
@@ -76,7 +116,10 @@ async function loadCollection(prefix: string): Promise<IconCollection | null> {
76116
if (collectionCache.has(prefix)) return collectionCache.get(prefix)!
77117

78118
const jsonPath = resolveCollectionPath(prefix)
79-
if (!jsonPath) return null
119+
if (!jsonPath) {
120+
warnMissingCollection(prefix)
121+
return null
122+
}
80123

81124
try {
82125
const data = await Bun.file(jsonPath).json()
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* Tests for locating an icon collection on disk.
3+
*
4+
* Regression focus: resolution only understood the monolithic `@iconify/json`
5+
* package (~120MB, every collection). Projects overwhelmingly install the
6+
* per-collection `@iconify-json/<prefix>` packages instead, and for those every
7+
* `<Icon>` on every page rendered as an empty string with no error — a blank
8+
* nav and blank buttons with nothing in the markup to explain it.
9+
*/
10+
11+
import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
12+
import { tmpdir } from 'node:os'
13+
import { join } from 'node:path'
14+
import process from 'node:process'
15+
import { afterEach, describe, expect, it } from 'bun:test'
16+
import { resolveCollectionPath } from '../src/builtins/icon'
17+
18+
const originalCwd = process.cwd()
19+
const roots: string[] = []
20+
21+
// realpath because macOS hands out /var/folders/… which is a symlink to
22+
// /private/var/folders/…, and `process.cwd()` reports the resolved form.
23+
function makeRoot(): string {
24+
const root = realpathSync(mkdtempSync(join(tmpdir(), 'stx-icons-')))
25+
roots.push(root)
26+
return root
27+
}
28+
29+
/** Write a stub collection at `relativePath` under `root`. */
30+
function writeCollection(root: string, relativePath: string): string {
31+
const full = join(root, relativePath)
32+
mkdirSync(join(full, '..'), { recursive: true })
33+
writeFileSync(full, JSON.stringify({ icons: { play: { body: '<path/>' } } }))
34+
return full
35+
}
36+
37+
afterEach(() => {
38+
process.chdir(originalCwd)
39+
for (const root of roots.splice(0)) {
40+
if (existsSync(root)) rmSync(root, { recursive: true, force: true })
41+
}
42+
})
43+
44+
describe('icon collection resolution', () => {
45+
it('finds a per-collection @iconify-json package in node_modules', () => {
46+
const root = makeRoot()
47+
const expected = writeCollection(root, 'node_modules/@iconify-json/lucide/icons.json')
48+
process.chdir(root)
49+
50+
expect(resolveCollectionPath('lucide')).toBe(expected)
51+
})
52+
53+
it('finds a per-collection package vendored in pantry', () => {
54+
const root = makeRoot()
55+
const expected = writeCollection(root, 'pantry/@iconify-json/lucide/icons.json')
56+
process.chdir(root)
57+
58+
expect(resolveCollectionPath('lucide')).toBe(expected)
59+
})
60+
61+
it('still finds the monolithic @iconify/json package', () => {
62+
const root = makeRoot()
63+
const expected = writeCollection(root, 'node_modules/@iconify/json/json/lucide.json')
64+
process.chdir(root)
65+
66+
expect(resolveCollectionPath('lucide')).toBe(expected)
67+
})
68+
69+
it('prefers the per-collection package when both are present', () => {
70+
const root = makeRoot()
71+
const perCollection = writeCollection(root, 'node_modules/@iconify-json/lucide/icons.json')
72+
writeCollection(root, 'node_modules/@iconify/json/json/lucide.json')
73+
process.chdir(root)
74+
75+
expect(resolveCollectionPath('lucide')).toBe(perCollection)
76+
})
77+
78+
it('returns null when the collection is installed under neither convention', () => {
79+
const root = makeRoot()
80+
process.chdir(root)
81+
82+
expect(resolveCollectionPath('definitely-not-a-collection')).toBeNull()
83+
})
84+
85+
it("prefers the project's own install over one vendored inside stx", () => {
86+
// `require.resolve` resolves relative to the stx module, so without the
87+
// cwd check first a collection sitting in stx's own dependencies would
88+
// shadow the one the app actually declared.
89+
const root = makeRoot()
90+
const projectCopy = writeCollection(root, 'node_modules/@iconify-json/hugeicons/icons.json')
91+
process.chdir(root)
92+
93+
expect(resolveCollectionPath('hugeicons')).toBe(projectCopy)
94+
})
95+
})

0 commit comments

Comments
 (0)