sf-symbols-lib: Comprehensive Code Review

sf-symbols-lib
12 February 2026
Reviewer: Claude Code (claude-opus-4-6)
Scope: All non-generated source files, scripts, configuration, and documentation
PAP-002

sf-symbols-lib is a well-structured React component library providing 7,007 Apple SF Symbols as individually tree-shakeable components in two variants, and the core library code is clean, well-typed, and production-ready. However, the project suffers from significant documentation drift, because the README and several scripts still reference the old 4-variant architecture despite the codebase having been refactored to 2 variants. There are also three entirely obsolete script files, duplicated utility functions across four files, and multiple short variable names that hurt readability.

Executive Summary

sf-symbols-lib is a well-structured React component library providing 7,007 Apple SF Symbols as individually tree-shakeable components in two variants (dualtone and monochrome). The core library code (src/common/) is clean, well-typed, and production-ready. However, the project suffers from significant documentation drift: the README and several scripts still reference the old 4-variant architecture (hierarchical/palette/multicolor) despite the codebase having been refactored to 2 variants (dualtone/monochrome). There are also three entirely obsolete script files, duplicated utility functions across four files, and multiple short variable names that hurt readability. Security posture is acceptable given the domain, though the use of dangerouslySetInnerHTML warrants documented justification.

Strengths

• Clean core architecture. The SFIcon.tsx / context.ts / types.ts trio is minimal, well-documented, and correctly uses forwardRef, context, and TypeScript generics. The separation between renderer and generated components is excellent.
• Tree-shaking design. Each icon is a standalone module with sideEffects: false in package.json and preserveModules: true in Vite config. Consumers only pay for what they import.
• Correct ESM-only publishing. The exports map in package.json properly separates type and module entry points with conditional exports for sub-paths.
• "use client" Vite plugin. The clientDirectivePlugin in vite.config.ts is a clean, minimal solution for RSC/Next.js App Router compatibility.
• TypeScript strictness. tsconfig.json enables strict, noUnusedLocals, noUnusedParameters, and noFallthroughCasesInSwitch.
• Good JSDoc coverage on core library types. SFIconProps, SFIconRenderProps, SFIconContext, resolveSize, and the size constants all have proper doc comments.
• Metadata cross-validation. The generator (generate-sfsymbols.ts:328-330) cross-validates the embedded lib name against the generated PascalCase name, catching naming mismatches early.

1. Redundancies

1.1 Duplicated kebabToPascalCase function

Severity: High

Files:

• scripts/generate-sfsymbols.ts:16-27
• scripts/generate-docs-data.ts:55-65
• scripts/generate-sfsymbols-test.ts:18-28
• scripts/generate-metadata.ts:21-31
• docs/scripts/utils.js:52-57
• docs/scripts/lazy-loader.js:310-319

Description: The kebabToPascalCase function is copy-pasted identically across four TypeScript scripts and reimplemented in two JavaScript files (6 total copies). The test generator version is identical to the main generator.

Code quote (all four TS versions are identical):

function kebabToPascalCase(kebabStr: string): string {
  const result = kebabStr
    .split(/[-.]/)
    .filter(word => word.length > 0)
    .map(word => {
      if (/^\d+$/.test(word)) return word;
      return word.charAt(0).toUpperCase() + word.slice(1);
    })
    .join('');
  return 'SF' + result;
}

Recommendation: Extract into a shared scripts/utils.ts module and import it in all scripts. The docs JS versions can remain separate (different runtime), but should also share via a single utility file.

1.2 Duplicated parseMetadataFromContent function

Severity: Medium

Files:

• scripts/generate-sfsymbols.ts:53-74
• scripts/generate-docs-data.ts:70-89

Description: Both scripts parse SVG metadata XML with nearly identical regex-based extraction. The generate-sfsymbols version returns SvgMetadata (with appleName and libName), while generate-docs-data returns SymbolMetadata (without those fields). The core parsing logic is identical.

Recommendation: Extract a shared parseMetadataFromSvg function into scripts/utils.ts. The caller can pick only the fields it needs.

1.3 Duplicated parseSvgFile function

Severity: Medium

Files:

• scripts/generate-sfsymbols.ts:79-105
• scripts/generate-docs-data.ts:94-113

Description: Both functions perform the same SVG parsing pipeline: read file, extract metadata, extract viewBox, strip XML preamble/comments/metadata, collapse whitespace, replace color fills, extract inner content. The logic is identical line-for-line.

Recommendation: Consolidate into a shared utility module. Both scripts should import the same parser.

1.4 Duplicated SVG content extraction in test generator

Severity: Low

File: scripts/generate-sfsymbols-test.ts:30-44

Description: extractViewBox and extractSvgContent duplicate the logic from parseSvgFile but read the file twice (once per function). The main generator reads the file once.

Recommendation: If the test generator is kept (see finding 4.1), refactor to use the shared parser.

2. Modularization Opportunities

2.1 Shared scripts utility module

Severity: High

Files: All four scripts under scripts/

Description: The scripts share at least three functions (kebabToPascalCase, parseMetadataFromContent, parseSvgFile) plus the VARIANTS constant and ensureDir helper. These should live in a shared module.

Recommendation: Create scripts/shared/svg-parser.ts and scripts/shared/naming.ts (or a single scripts/shared/utils.ts), then import from each script.

2.2 Generator template extraction

Severity: Low

File: scripts/generate-sfsymbols.ts:121-158

Description: generateIconComponentFile builds the generated component code as a template literal. This is fine for a single template, but if additional variants or component styles are added in the future, extracting the template into a separate file would improve maintainability.

Recommendation: No action needed now, but consider extraction if a third variant is added.

3. Dead/Unused Code

3.1 Unused shortcutsHtml variable in HTML template

Severity: Low

File: scripts/generate-docs-data.ts:291

Description: shortcutsHtml is read from shortcuts.md and used to replace content in modals.js, but there is no <!-- SHORTCUTS_CONTENT --> placeholder in the HTML template being replaced at line 293-294. The HTML replacements at lines 293-294 only handle ABOUT_CONTENT and SEARCH_CONTENT. The shortcuts injection only works via modals.js (lines 319-324).

Code:

htmlContent = htmlContent.replace('<!-- ABOUT_CONTENT -->', aboutHtml);
htmlContent = htmlContent.replace('<!-- SEARCH_CONTENT -->', searchHtml);
// shortcutsHtml is NOT injected into htmlContent

The console log at line 295 says "Injected markdown content (about.md, search.md)" which is correct, but shortcutsHtml is still read at line 291 for the modals.js injection, so it is technically used. However, the HTML template replacement for search may also be unnecessary if modals.js already handles it via JS injection (lines 319-324 handle all three).

Recommendation: Verify whether the <!-- SEARCH_CONTENT --> and <!-- ABOUT_CONTENT --> placeholders in the HTML template are actually needed given that modals.js also injects them. If both paths are needed (SSR fallback + JS enhancement), document the reasoning.

3.2 as ReactElement type assertion in SFIcon

Severity: Info

File: src/common/SFIcon.tsx:62

Description: The as ReactElement assertion on the JSX return value is unnecessary. TypeScript already infers the correct return type from JSX.

Code:

    ) as ReactElement;

Recommendation: Remove the type assertion. The return type is already correctly inferred.

4. Unused Files

4.1 scripts/generate-sfsymbols-test.ts: Obsolete test generator

Severity: High

File: scripts/generate-sfsymbols-test.ts

Description: This file generates 10 test icons using the old architecture: it references hierarchical variant (line 9), generates per-icon Props interfaces and SIZE_MAP constants (the current architecture uses shared SFIconProps and SF_ICON_SIZES), imports React directly (current uses JSX transform), and uses <g dangerouslySetInnerHTML> instead of the current <svg dangerouslySetInnerHTML> pattern. It is not referenced by any package.json script or import.

The to-dos.md file explicitly lists "Remove obsolete generate-sfsymbols-test.ts" as a medium priority task.

Recommendation: Delete this file.

4.2 scripts/generate-metadata.ts: Obsolete metadata generator

Severity: High

File: scripts/generate-metadata.ts

Description: This file references the old hierarchical variant (line 13), uses a heuristic-based extractCategory function instead of the real metadata categories, and writes to docs/data/ (a directory that does not exist in the current project structure). It is not referenced by any package.json script. The to-dos.md file also lists this for removal.

Recommendation: Delete this file.

4.3 scripts/minify-dist.js: Unreferenced minification script

Severity: Medium

File: scripts/minify-dist.js

Description: This script minifies JS/CSS files in docs/dist/. It is not referenced by any npm script in package.json. The generate-docs-data.ts script already handles CSS minification inline via clean-css. It is unclear when or how this script is invoked.

Recommendation: Either add it to the docs build pipeline (e.g., docs:minify script) or delete it. If kept, it duplicates CSS minification logic already in generate-docs-data.ts.

4.4 docs/scripts/lazy-loader.js: Unused virtual scrolling module

Severity: Medium

File: docs/scripts/lazy-loader.js

Description: This 359-line LazyIconLoader class with virtual scrolling is never imported by any other file. main.js imports from symbols.js, data.js, utils.js, theme.js, modals.js, and colors.js, but never lazy-loader.js. It appears to be an unused prototype for a virtual scrolling approach that was never integrated.

Recommendation: Delete this file or integrate it. The current docs site renders all 7,007 icons as DOM elements (noted in to-dos.md as a performance issue).

4.5 session.md: Stale session log

Severity: Low

File: session.md

Description: This is a 1,722+ line dump of a previous Claude Code session's tool calls and outputs, including npm commands, git operations, and version bumping. It contains no useful code or documentation and is not gitignored.

Recommendation: Delete this file or add it to .gitignore.

4.6 sf-symbols-lib-fixes.md: Stale build fixes document

Severity: Low

File: sf-symbols-lib-fixes.md

Description: This document describes 5 build fixes for v1.0.2. All fixes have been applied (as confirmed in the session.md). The document serves no ongoing purpose and is not gitignored.

Recommendation: Delete this file or move it to an archive directory.

5. Documentation Gaps

5.1 README references non-existent variants

Severity: Critical

File: README.md

Description: The README references 4 variants (hierarchical, monochrome, palette, multicolor) throughout, but the codebase was refactored to only support 2 variants (dualtone, monochrome). All import examples use sf-symbols-lib/hierarchical which no longer exists. The to-dos.md explicitly lists this as a high priority task.

Specific outdated references:

• Line 14: "4 Variants - Hierarchical, Monochrome, Palette, Multicolor"
• Lines 32, 51, 70, 87, 93, 122, 143, 169, 230: Import from sf-symbols-lib/hierarchical
• Lines 76, 79: Import from sf-symbols-lib/palette and sf-symbols-lib/multicolor
• Lines 286-299: Project structure shows hierarchical/palette/multicolor directories

Recommendation: Update all import paths from hierarchical to dualtone, remove palette/multicolor references, change "4 Variants" to "2 Variants".

5.2 MIGRATION.md references non-existent variants and compat layer

Severity: High

File: MIGRATION.md

Description: The migration guide references sf-symbols-lib/hierarchical throughout and mentions a compat layer (sf-symbols-lib/compat) that was removed (confirmed in to-dos.md completed items: "Compat Layer Removed"). Import paths, code examples, and the compat layer documentation are all outdated.

Recommendation: Update all import paths to use dualtone/monochrome and remove or mark the compat layer section as removed.

5.3 NPM Commands.md is outdated

Severity: Medium

File: NPM Commands.md

Description: The clean script description references src/hierarchical src/monochrome src/components/sf-symbol-name.ts but the current package.json clean script references src/dualtone src/monochrome. The build command description is missing the tsc-alias step. The docs commands are incomplete (missing docs:minify).

Recommendation: Regenerate this file from the current package.json scripts.

5.4 README size presets table is incorrect

Severity: Medium

File: README.md:113-119

Description: The README's size presets table shows different pixel values than the actual code:

Every value is wrong.

Code quote (types.ts:16-22):

export const SF_ICON_SIZES: Record<SFIconSizePreset, number> = {
  xs: 12,
  sm: 16,
  md: 20,
  lg: 24,
  xl: 32,
};

Recommendation: Update the README table to match the actual values.

5.5 README Props table default size is incorrect

Severity: Medium

File: README.md:183

Description: The props table says the default size is 'md', but SFIcon.tsx:41 defaults to 'lg':

const { size = context.size ?? 'lg', ... } = props;

Recommendation: Update the README to show the correct default of 'lg' (24px).

5.6 Missing JSDoc on ESLint config

Severity: Info

File: eslint.config.js

Description: The ESLint configuration file has no top-level comment explaining the configuration choices, particularly why @typescript-eslint/no-explicit-any and no-useless-escape are disabled.

Recommendation: Add a brief comment for each non-standard rule override.

6. Constants Consolidation

6.1 Hardcoded theme colors in docs scripts

Severity: Medium

Files:

• docs/scripts/colors.js:108: '#adbac7' and '#24292e'
• docs/scripts/theme.js:43-46: Inline Bootstrap icon HTML with font-size:18px

Description: The dark/light theme foreground colors are hardcoded in colors.js but defined as CSS variables in the stylesheets. The inline icon styles in theme.js duplicate size/style values.

Recommendation: Read theme colors from CSS variables at runtime instead of hardcoding hex values. Move inline icon styles to CSS classes.

6.2 Variant list duplication

Severity: Low

Files:

• scripts/generate-sfsymbols.ts:6: const VARIANTS = ['dualtone', 'monochrome'] as const;
• scripts/generate-docs-data.ts:50: const VARIANTS: readonly Variant[] = ['dualtone', 'monochrome'];
• scripts/generate-sfsymbols-test.ts:9: const VARIANTS = ['hierarchical', 'monochrome'] as const; (stale!)
• scripts/generate-metadata.ts:13: const VARIANTS = ['hierarchical', 'monochrome'] as const; (stale!)

Description: The variant list is defined independently in every script. Two of them still reference the old hierarchical variant.

Recommendation: After creating a shared utils module (finding 2.1), export VARIANTS from there as the single source of truth.

6.3 Magic number for chunk size

Severity: Low

File: scripts/generate-docs-data.ts:160

Description: const CHUNK_SIZE = 500; is defined inline inside the writeChunks function.

Recommendation: Move to a top-level constant or configuration object.

7. Short Variable Names

7.1 Scripts and library code

7.2 Docs scripts (vanilla JS)

Recommendation: Rename all single/two-character variables to self-explanatory names, especially e/ev (always use event or error), p/f/t (use descriptive names), and loop indices (use index or descriptive names in non-trivial loops).

8. Architecture & Design

8.1 Generated code uses @/ path aliases

Severity: Medium

File: scripts/generate-sfsymbols.ts:139-140 (template)

Description: The generated icon components use @/common/SFIcon and @/common/types path aliases. This requires tsc-alias post-processing during build to rewrite them to relative paths in .d.ts files. While this works, it adds a build step and a dependency that would be unnecessary if the generated code used relative paths directly.

Generated code template:

import { SFIcon } from '@/common/SFIcon';
import { type SFIconProps } from '@/common/types';

Recommendation: Consider generating relative imports instead (e.g., ../../common/SFIcon). This would eliminate the tsc-alias dependency for the generated files. The path alias is nice for hand-written code but adds complexity for generated code.

8.2 Generated barrel files re-export using @/ aliases

Severity: Medium

File: scripts/generate-sfsymbols.ts:171

Description: The barrel export template uses @/${variant}/icons/${name} which also depends on tsc-alias. Same concern as 8.1.

Recommendation: Consider using relative paths in generated code.

8.3 No test infrastructure

Severity: High

File: package.json

Description: There are no testing dependencies (vitest, jest, @testing-library/react), no test scripts, and no test files. For a published npm package with 14,000+ generated components, there is no automated verification that:

• Generated components render correctly
• The SFIcon renderer produces valid SVG output
• resolveSize returns correct values
• Context inheritance works properly
• forwardRef passes refs correctly

Recommendation: Add vitest (or jest) with @testing-library/react. At minimum, test the core SFIcon component, resolveSize, context defaults, and a sample generated icon.

8.4 Docs site still references 4 variants in data structures

Severity: High

Files:

• docs/scripts/data.js:17-18: { hierarchical: {}, monochrome: {}, palette: {}, multicolor: {} }
• docs/scripts/data.js:29: Same 4-variant structure for chunksLoaded
• docs/scripts/symbols.js:24: Default rendering mode 'hierarchical'
• docs/scripts/symbols.js:28-36: processMulticolorContent function

Description: The docs site JavaScript still initializes data structures for 4 variants and has a processMulticolorContent function for a variant that no longer exists. While the docs are fed data from meta.json at runtime, the hardcoded defaults and dead code paths are confusing.

Recommendation: Update data.js to only list dualtone and monochrome. Remove processMulticolorContent. Change default rendering mode to 'dualtone'.

8.5 globals output in Vite config is unnecessary for ESM-only

Severity: Info

File: vite.config.ts:49-52

Description: The globals mapping (react: 'React', 'react-dom': 'ReactDOM') is only relevant for UMD/IIFE formats. Since the library is ESM-only (formats: ['es']), this configuration has no effect.

globals: {
  react: 'React',
  'react-dom': 'ReactDOM'
},

Recommendation: Remove the globals block for clarity, or add a comment explaining it is kept for potential future UMD builds.

9. Security Considerations

9.1 dangerouslySetInnerHTML with generated SVG content

Severity: Medium

File: src/common/SFIcon.tsx:60

Description: The SFIcon component uses dangerouslySetInnerHTML={{ __html: svgContent }} to render SVG paths. The svgContent comes from auto-generated component files where the SVG data is baked in as a string literal at code generation time.

Risk assessment:

• The content is NOT user-supplied at runtime. It is generated from source SVGs during build.
• The generator (generate-sfsymbols.ts:127-131) escapes backslashes, single quotes, newlines, and collapses whitespace, but does NOT sanitize for HTML/SVG injection.
• If the source SVGs (in .svgs/) contained malicious SVG (e.g., <script>, <foreignObject>, onload attributes), it would be embedded verbatim into the generated components.

Recommendation:

1. Add a comment in SFIcon.tsx explaining why dangerouslySetInnerHTML is safe (content is build-time generated, not runtime).
2. Consider adding a sanitization step in the generator that strips <script>, <foreignObject>, and event handler attributes from SVG content.
3. The SFIconProps interface already correctly omits dangerouslySetInnerHTML from the public API (Omit<SVGAttributes<SVGSVGElement>, 'dangerouslySetInnerHTML'> in types.ts:31), which is good.

9.2 Template injection in docs generator

Severity: Low

File: scripts/generate-docs-data.ts:315-324

Description: Markdown content is converted to HTML and injected into JavaScript files via string replacement. The escaping only handles backticks:

const escapedAboutHtml = aboutHtml.replace(/`/g, '\\`');

If the markdown source contained ${...} template expressions, they could be evaluated as JavaScript when the string is used in a template literal context.

Recommendation: Also escape $ characters: .replace(/\$/g, '\\$'). Low risk since the markdown files are developer-controlled, but defense-in-depth is good practice.

9.3 execSync in generator script

Severity: Low

File: scripts/generate-sfsymbols.ts:355

Description: execSync('tsx ./scripts/generate-docs-data.ts', { stdio: 'inherit' }) uses a hardcoded command string. This is safe as-is, but would be vulnerable to path injection if any part were dynamic.

Recommendation: No action needed, but consider using the programmatic API (import and call the function directly) instead of spawning a subprocess. This would also improve error handling and performance.

10. Coding Standards

10.1 ESLint disables no-explicit-any

Severity: Medium

File: eslint.config.js:31

Description: @typescript-eslint/no-explicit-any is disabled globally. This undermines TypeScript's type safety across the entire project.

'@typescript-eslint/no-explicit-any': 'off',

Recommendation: Re-enable the rule (at least as warn) and fix any violations. The core library code does not appear to use any anywhere, so this may be a leftover from earlier development.

10.2 ESLint disables no-useless-escape

Severity: Low

File: eslint.config.js:32

Description: no-useless-escape is disabled globally. This allows unnecessary escape characters in strings and regexes.

Recommendation: Re-enable and fix any flagged escapes.

10.3 ESLint pathGroups reference non-existent aliases

Severity: Low

File: eslint.config.js:44-59

Description: The import ordering configuration defines path groups for @shared/**, @backend/**, and @frontend/**, none of which exist in this project. Only @/* is used (mapped to src/*).

Recommendation: Remove the unused path groups and add one for @/*.

10.4 moduleResolution: "node" instead of "bundler"

Severity: Low

File: tsconfig.json:8

Description: The project uses "moduleResolution": "node" with ESM ("module": "ESNext"). For Vite-based projects, "moduleResolution": "bundler" is the recommended setting as it correctly handles exports fields in package.json and conditional imports.

Recommendation: Change to "moduleResolution": "bundler". This may also allow removing allowSyntheticDefaultImports and esModuleInterop as they become the default behavior.

10.5 Mixed fs import styles in docs generator

Severity: Low

File: scripts/generate-docs-data.ts:6-7

Description: The file imports both fsSync (synchronous) and fs (promises) but uses fsSync for simple operations that could be async:

import fsSync from 'fs';
import fs from 'fs/promises';

Lines 95, 202, 222, 230 use fsSync.readFileSync and fsSync.existsSync within an already-async main() function.

Recommendation: Use await fs.readFile() and await fs.access() (or try/catch) consistently for operations inside async functions. Reserve synchronous operations for the parseSvgFile function if performance is a concern for the 7,007 iterations.

10.6 Generated icon template uses short variable names

Severity: Medium

File: scripts/generate-sfsymbols.ts:142-143 (template output)

Description: The generated icon component files use abbreviated constants:

const SVG = '...';
const VB = '0 0 24 24';

These are generated constants in auto-generated files, so human readability is less critical. However, SVG and VB are unnecessarily cryptic.

Recommendation: Use SVG_CONTENT and VIEW_BOX in the generated template for clarity.

10.7 Inconsistent error handling pattern

Severity: Low

Files:

• scripts/generate-sfsymbols.ts:360-363: .catch() with process.exit(1)
• scripts/generate-docs-data.ts:340-343: try/catch inside main() with process.exit(1)
• scripts/generate-docs-data.ts:347-350: .catch() with process.exit(1) (double exit path)

Description: generate-docs-data.ts has both a try/catch inside main() AND a .catch() on the main() call, creating redundant error handling. The generator script only uses .catch().

Recommendation: Pick one pattern and use it consistently. The .catch() on the top-level call is sufficient if main() is properly async.

Summary Table

By Category

Prioritized Action Items

1. Update README.md. Fix all variant references, size preset values, and default size (Critical + 2 Medium)
2. Delete obsolete scripts. generate-sfsymbols-test.ts, generate-metadata.ts (2 High)
3. Update MIGRATION.md. Fix variant references, remove compat layer docs (High)
4. Extract shared utilities. Create scripts/shared/utils.ts for duplicated functions (High + Medium)
5. Add test infrastructure. vitest + basic tests for core components (High)
6. Update docs site JS. Remove 4-variant references, update defaults to dualtone (High)
7. Clean up stale files. session.md, sf-symbols-lib-fixes.md, lazy-loader.js, minify-dist.js (4 Medium/Low)
8. Rename short variables. Systematic pass through all files (Medium)
9. Re-enable ESLint rules. no-explicit-any, no-useless-escape (Medium + Low)