diff --git a/.changeset/octicons-react-codesplitting.md b/.changeset/octicons-react-codesplitting.md new file mode 100644 index 000000000..fb102af83 --- /dev/null +++ b/.changeset/octicons-react-codesplitting.md @@ -0,0 +1,5 @@ +--- +"@primer/octicons-react": minor +--- + +Optimize `@primer/octicons-react` for codesplitting and tree-shaking. Each icon is now emitted as its own module and exposed via a `./*` subpath export, so icons can be dynamically imported and code-split (e.g. `import('@primer/octicons-react/AlertIcon')`). The generated icons are now finished `React.forwardRef` components built on a shared `renderOcticon` runtime instead of runtime `createIconComponent` factory calls. Existing `import {AlertIcon}` and `import * as Octicons` usage continues to work unchanged. diff --git a/lib/octicons_react/__tests__/__fixtures__/dynamic-imports.mjs b/lib/octicons_react/__tests__/__fixtures__/dynamic-imports.mjs new file mode 100644 index 000000000..19d2cbaee --- /dev/null +++ b/lib/octicons_react/__tests__/__fixtures__/dynamic-imports.mjs @@ -0,0 +1,12 @@ +// Fixture for the code-splitting test: static dynamic imports of per-icon +// subpaths so Rollup emits one chunk per icon. +export function load(name) { + switch (name) { + case 'AlertIcon': + return import('../../dist/icons/AlertIcon.mjs') + case 'RepoIcon': + return import('../../dist/icons/RepoIcon.mjs') + default: + return null + } +} diff --git a/lib/octicons_react/__tests__/codesplitting.test.js b/lib/octicons_react/__tests__/codesplitting.test.js new file mode 100644 index 000000000..0b43f33e5 --- /dev/null +++ b/lib/octicons_react/__tests__/codesplitting.test.js @@ -0,0 +1,50 @@ +const path = require('node:path') +const fs = require('node:fs') +const {rollup} = require('rollup') + +const packageRoot = path.resolve(__dirname, '..') +const iconsDir = path.join(packageRoot, 'dist', 'icons') + +test('emits one pre-transformed module per icon', () => { + const alert = fs.readFileSync(path.join(iconsDir, 'AlertIcon.mjs'), 'utf8') + // The generated icon is a finished forwardRef component using the shared + // renderOcticon runtime, not a runtime createIconComponent factory call. + expect(alert).toContain('React.forwardRef') + expect(alert).toContain('renderOcticon(') + expect(alert).not.toContain('createIconComponent') + // Per-icon declaration file is emitted alongside for subpath type resolution. + expect(fs.existsSync(path.join(iconsDir, 'AlertIcon.d.ts'))).toBe(true) +}) + +test('the barrel is a pure re-export of the per-icon modules', () => { + const barrel = fs.readFileSync(path.join(packageRoot, 'dist', 'index.esm.mjs'), 'utf8') + expect(barrel).toContain("export { AlertIcon } from './icons/AlertIcon.mjs'") + // A pure re-export barrel contains no rendering logic of its own. + expect(barrel).not.toContain('forwardRef') +}) + +test('package.json exports expose the "." barrel and a per-icon "./*" subpath', () => { + const pkg = require(path.join(packageRoot, 'package.json')) + expect(pkg.exports['.'].import).toBe('./dist/index.esm.mjs') + expect(pkg.exports['.'].require).toBe('./dist/index.umd.js') + expect(pkg.exports['./*'].import).toBe('./dist/icons/*.mjs') + expect(pkg.exports['./*'].types).toBe('./dist/icons/*.d.ts') +}) + +test('dynamic subpath imports are code-split into separate chunks', async () => { + const bundle = await rollup({ + input: path.join(__dirname, '__fixtures__', 'dynamic-imports.mjs'), + external: ['react'] + }) + const {output} = await bundle.generate({format: 'esm'}) + + // The entry plus one chunk per dynamically imported icon (and any shared + // runtime chunk). More than a single chunk proves the icons are code-split + // rather than bundled into the entry. + expect(output.length).toBeGreaterThan(1) + + const entry = output.find(chunk => chunk.isEntry) + // The entry itself must not inline any icon path data. + expect(entry.code).not.toContain('octicon octicon-alert') + expect(entry.code).not.toContain('octicon octicon-repo') +}) diff --git a/lib/octicons_react/__tests__/tree-shaking.test.js b/lib/octicons_react/__tests__/tree-shaking.test.js index 32fd93117..f6ba7f841 100644 --- a/lib/octicons_react/__tests__/tree-shaking.test.js +++ b/lib/octicons_react/__tests__/tree-shaking.test.js @@ -50,5 +50,5 @@ test('tree shaking single export', async () => { }) const bundleSize = Buffer.byteLength(output[0].code.trim()) / 1000 - expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"6.119kB"`) + expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"6.309kB"`) }) diff --git a/lib/octicons_react/package.json b/lib/octicons_react/package.json index 8037d23ba..fc92bac14 100644 --- a/lib/octicons_react/package.json +++ b/lib/octicons_react/package.json @@ -9,12 +9,18 @@ "main": "dist/index.umd.js", "module": "dist/index.esm.mjs", "exports": { - "types": { - "import": "./dist/index.d.mts", - "require": "./dist/index.d.ts" + ".": { + "types": { + "import": "./dist/index.d.mts", + "require": "./dist/index.d.ts" + }, + "import": "./dist/index.esm.mjs", + "require": "./dist/index.umd.js" }, - "import": "./dist/index.esm.mjs", - "require": "./dist/index.umd.js" + "./*": { + "types": "./dist/icons/*.d.ts", + "import": "./dist/icons/*.mjs" + } }, "sideEffects": false, "types": "dist/index.d.ts", diff --git a/lib/octicons_react/rollup.config.js b/lib/octicons_react/rollup.config.js index 23cfa253a..0276e1567 100644 --- a/lib/octicons_react/rollup.config.js +++ b/lib/octicons_react/rollup.config.js @@ -1,3 +1,5 @@ +import fs from 'fs' +import path from 'path' import babel from '@rollup/plugin-babel' import commonjs from '@rollup/plugin-commonjs' import packageJson from './package.json' @@ -12,37 +14,54 @@ function createPackageRegex(name) { return new RegExp(`^${name}(/.*)?`) } -const baseConfig = { - input: 'src/index.js', - external: dependencies.map(createPackageRegex), - plugins: [ - babel({ - babelrc: false, - presets: [ - [ - '@babel/preset-env', - { - modules: false - } - ], - '@babel/preset-react' - ], - babelHelpers: 'bundled' - }), - commonjs() - ] -} +const iconsDir = path.resolve(__dirname, 'src/__generated__/icons') + +// One entry per generated icon module (plus the barrel) so `dist/` mirrors the +// source tree: `dist/index.esm.mjs`, `dist/icons/AlertIcon.mjs`, etc. This +// enables `import('@primer/octicons-react/AlertIcon')` codesplitting while the +// barrel keeps existing named imports working and tree-shakeable. +const iconInputs = Object.fromEntries( + fs + .readdirSync(iconsDir) + .filter(file => file.endsWith('.js') && file !== 'index.js') + .map(file => [`icons/${path.basename(file, '.js')}`, path.join(iconsDir, file)]) +) + +const babelPlugin = babel({ + babelrc: false, + presets: [ + [ + '@babel/preset-env', + { + modules: false + } + ], + '@babel/preset-react' + ], + babelHelpers: 'bundled' +}) + +const external = dependencies.map(createPackageRegex) export default [ { - ...baseConfig, + input: { + 'index.esm': 'src/index.js', + ...iconInputs + }, + external, + plugins: [babelPlugin, commonjs()], output: { - file: `dist/index.esm.mjs`, - format: 'esm' + dir: 'dist', + format: 'esm', + entryFileNames: '[name].mjs', + chunkFileNames: '[name]-[hash].mjs' } }, { - ...baseConfig, + input: 'src/index.js', + external, + plugins: [babelPlugin, commonjs()], output: { file: `dist/index.umd.js`, format: 'umd', diff --git a/lib/octicons_react/script/build.js b/lib/octicons_react/script/build.js index b333bfc87..2213e6d19 100755 --- a/lib/octicons_react/script/build.js +++ b/lib/octicons_react/script/build.js @@ -7,8 +7,7 @@ const fse = require('fs-extra') const {join, resolve} = require('path') const srcDir = resolve(__dirname, '../src/__generated__') -const iconsFile = join(srcDir, 'icons.js') -const typesFile = join(srcDir, 'icons.d.ts') +const iconsDir = join(srcDir, 'icons') const GENERATED_HEADER = '/* THIS FILE IS GENERATED. DO NOT EDIT IT. */' @@ -19,6 +18,7 @@ function pascalCase(str) { const icons = Object.entries(octicons) .map(([key, octicon]) => { const name = `${pascalCase(key)}Icon` + const heights = Object.keys(octicon.heights) // Build an object with the following structure: // // type SVGData = { @@ -39,61 +39,93 @@ const icons = Object.entries(octicons) ) }) ) - // Define the icon by using the `createIconComponent` helper and the svgData - // defined above. This generates the following: + + // Emit a finished, pre-transformed component instead of a runtime + // `createIconComponent` factory call. Everything the factory used to do at + // module-eval time (reading the SVG data, computing the list of heights, + // wrapping in `forwardRef`) is statically known and emitted directly: // - // const IconName = /*#__PURE__*/ createIconComponent('name', 'default-class-name', () => ({ - // /* svgData */ - // })); + // const heights = ["16"] + // const svgDataByHeight = { "16": { width: 16, path: } } + // export const IconName = /*#__PURE__*/ React.forwardRef((props, ref) => + // renderOcticon(props, ref, 'octicon octicon-name', svgDataByHeight, heights) + // ) + // IconName.displayName = "IconName" // - const {code} = generate( - t.variableDeclaration('const', [ - t.variableDeclarator( - t.identifier(name), - t.addComment( - t.callExpression(t.identifier('createIconComponent'), [ - // The name of the generated icon - t.stringLiteral(name), - // The className used on the underlying element - t.stringLiteral(`octicon octicon-${key}`), - t.arrowFunctionExpression([], t.blockStatement([t.returnStatement(svgData)])) - ]), - 'leading', - '#__PURE__' - ) + const forwardRefCall = t.addComment( + t.callExpression(t.memberExpression(t.identifier('React'), t.identifier('forwardRef')), [ + t.arrowFunctionExpression( + [t.identifier('props'), t.identifier('ref')], + t.callExpression(t.identifier('renderOcticon'), [ + t.identifier('props'), + t.identifier('ref'), + t.stringLiteral(`octicon octicon-${key}`), + t.identifier('svgDataByHeight'), + t.identifier('heights') + ]) ) - ]) + ]), + 'leading', + '#__PURE__' ) + const program = t.program([ + t.importDeclaration([t.importDefaultSpecifier(t.identifier('React'))], t.stringLiteral('react')), + t.importDeclaration( + [t.importSpecifier(t.identifier('renderOcticon'), t.identifier('renderOcticon'))], + t.stringLiteral('../../renderOcticon') + ), + t.variableDeclaration('const', [ + t.variableDeclarator(t.identifier('heights'), t.arrayExpression(heights.map(height => t.stringLiteral(height)))) + ]), + t.variableDeclaration('const', [t.variableDeclarator(t.identifier('svgDataByHeight'), svgData)]), + t.exportNamedDeclaration( + t.variableDeclaration('const', [t.variableDeclarator(t.identifier(name), forwardRefCall)]) + ), + t.expressionStatement( + t.assignmentExpression( + '=', + t.memberExpression(t.identifier(name), t.identifier('displayName')), + t.stringLiteral(name) + ) + ) + ]) + + const {code} = generate(program) + return { key, name, octicon, - code + code: `${GENERATED_HEADER}\n${code}\n` } }) .sort((a, b) => a.key.localeCompare(b.key)) -function writeIcons(file) { +function writeIcons() { const count = icons.length - const code = `${GENERATED_HEADER} -import React from 'react' -import { createIconComponent } from '../createIconComponent' -${icons.map(({code}) => code).join('\n')} + // One module per icon so consumers can codesplit / dynamically import icons. + const iconWrites = icons.map(({name, code}) => fse.writeFile(join(iconsDir, `${name}.js`), code, 'utf8')) -export { - ${icons.map(({name}) => name).join(',\n ')} -}` - return fse.writeFile(file, code, 'utf8').then(() => { - console.warn('wrote %s with %d exports', file, count) + // A pure re-export barrel. Combined with `"sideEffects": false` and the + // `/*#__PURE__*/`-annotated per-icon modules, static named imports + // (`import {AlertIcon}`) tree-shake down to a single icon. + const barrel = `${GENERATED_HEADER} +${icons.map(({name}) => `export {${name}} from './${name}'`).join('\n')} +` + + return Promise.all([...iconWrites, fse.writeFile(join(iconsDir, 'index.js'), barrel, 'utf8')]).then(() => { + console.warn('wrote %d icon modules + barrel to %s', count, iconsDir) return icons }) } -function writeTypes(file) { +function writeTypes() { const count = icons.length - const code = `${GENERATED_HEADER} + + // Shared types, imported by each per-icon declaration file. + const sharedTypes = `${GENERATED_HEADER} import * as React from 'react' type Size = 'small' | 'medium' | 'large' @@ -108,23 +140,41 @@ interface IconProps { type Icon = React.FC -${icons.map(({name}) => `declare const ${name}: Icon`).join('\n')} +export {Icon, IconProps} +` + + // Per-icon declaration file so subpath imports (`import('.../AlertIcon')`) + // resolve their own types. + const typeWrites = icons.map(({name}) => { + const dts = `${GENERATED_HEADER} +import {Icon} from './types' + +declare const ${name}: Icon + +export {${name}} +` + return fse.writeFile(join(iconsDir, `${name}.d.ts`), dts, 'utf8') + }) -export { - Icon, - IconProps, - ${icons.map(({name}) => name).join(',\n ')} -}` - return fse.writeFile(file, code, 'utf8').then(() => { - console.warn('wrote %s with %d exports', file, count) + const barrel = `${GENERATED_HEADER} +export {Icon, IconProps} from './types' +${icons.map(({name}) => `export {${name}} from './${name}'`).join('\n')} +` + + return Promise.all([ + fse.writeFile(join(iconsDir, 'types.d.ts'), sharedTypes, 'utf8'), + ...typeWrites, + fse.writeFile(join(iconsDir, 'index.d.ts'), barrel, 'utf8') + ]).then(() => { + console.warn('wrote %d icon type modules + barrel to %s', count, iconsDir) return icons }) } fse - .mkdirs(srcDir) - .then(() => writeIcons(iconsFile)) - .then(() => writeTypes(typesFile)) + .emptyDir(iconsDir) + .then(() => writeIcons()) + .then(() => writeTypes()) .catch(error => { console.error(error) process.exit(1) diff --git a/lib/octicons_react/script/types.js b/lib/octicons_react/script/types.js index 43cecec2a..174cf2983 100755 --- a/lib/octicons_react/script/types.js +++ b/lib/octicons_react/script/types.js @@ -3,15 +3,20 @@ const fse = require('fs-extra') const {join, resolve} = require('path') const srcDir = resolve(__dirname, '../src/__generated__') -const iconsSrc = join(srcDir, 'icons.d.ts') +const iconsSrcDir = join(srcDir, 'icons') const indexSrc = join(srcDir, '../index.d.ts') const destDir = resolve(__dirname, '../dist') -const iconsDest = join(destDir, 'icons.d.ts') +const iconsDestDir = join(destDir, 'icons') const indexDest = join(destDir, 'index.d.ts') async function main() { - await fse.copy(iconsSrc, iconsDest) + // Copy only the generated declaration files (`.d.ts`) into `dist/icons`, so + // subpath imports resolve their own types. The `.js` sources are compiled to + // `.mjs` by Rollup and must not be copied here. + await fse.copy(iconsSrcDir, iconsDestDir, { + filter: src => fse.statSync(src).isDirectory() || src.endsWith('.d.ts') + }) let contents = await fse.readFile(indexSrc, 'utf8') contents = contents.replace(/.\/__generated__\//g, './') diff --git a/lib/octicons_react/src/createIconComponent.js b/lib/octicons_react/src/createIconComponent.js index 89e1e97f7..69c018757 100644 --- a/lib/octicons_react/src/createIconComponent.js +++ b/lib/octicons_react/src/createIconComponent.js @@ -1,78 +1,15 @@ import React from 'react' - -const sizeMap = { - small: 16, - medium: 32, - large: 64 -} +import {renderOcticon} from './renderOcticon' export function createIconComponent(name, defaultClassName, getSVGData) { const svgDataByHeight = getSVGData() const heights = Object.keys(svgDataByHeight) - const Icon = React.forwardRef( - ( - { - 'aria-label': ariaLabel, - 'aria-labelledby': arialabelledby, - tabIndex, - className = '', - fill = 'currentColor', - size = 16, - verticalAlign = 'text-bottom', - id, - title, - style, - ...rest - }, - forwardedRef - ) => { - const height = sizeMap[size] || size - const naturalHeight = closestNaturalHeight(heights, height) - const naturalWidth = svgDataByHeight[naturalHeight].width - const width = height * (naturalWidth / naturalHeight) - const path = svgDataByHeight[naturalHeight].path - const labelled = ariaLabel || arialabelledby - const role = labelled ? 'img' : undefined - - return ( - = 0 ? 'true' : 'false'} - aria-label={ariaLabel} - aria-labelledby={arialabelledby} - className={`${defaultClassName} ${className}`.trim()} - role={role} - viewBox={`0 0 ${naturalWidth} ${naturalHeight}`} - width={width} - height={height} - fill={fill} - id={id} - display="inline-block" - overflow="visible" - style={{ - verticalAlign, - ...style - }} - > - {title ? {title} : null} - {path} - - ) - } + const Icon = React.forwardRef((props, forwardedRef) => + renderOcticon(props, forwardedRef, defaultClassName, svgDataByHeight, heights) ) Icon.displayName = name return Icon } - -function closestNaturalHeight(naturalHeights, height) { - return naturalHeights - .map(naturalHeight => parseInt(naturalHeight, 10)) - .reduce((acc, naturalHeight) => (naturalHeight <= height ? naturalHeight : acc), naturalHeights[0]) -} diff --git a/lib/octicons_react/src/index.d.ts b/lib/octicons_react/src/index.d.ts index 376f14af5..1dcff903b 100644 --- a/lib/octicons_react/src/index.d.ts +++ b/lib/octicons_react/src/index.d.ts @@ -2,7 +2,7 @@ import * as React from 'react' // eslint-disable-next-line prettier/prettier -import {Icon} from './__generated__/icons.js' +import {Icon} from './__generated__/icons/index.js' type Size = 'small' | 'medium' | 'large' @@ -21,4 +21,4 @@ export interface OcticonProps extends React.ComponentPropsWithoutRef<'svg'> { verticalAlign?: 'middle' | 'text-bottom' | 'text-top' | 'top' | 'unset' } -export * from './__generated__/icons.js' +export * from './__generated__/icons/index.js' diff --git a/lib/octicons_react/src/renderOcticon.js b/lib/octicons_react/src/renderOcticon.js new file mode 100644 index 000000000..b8cdde54c --- /dev/null +++ b/lib/octicons_react/src/renderOcticon.js @@ -0,0 +1,75 @@ +import React from 'react' + +const sizeMap = { + small: 16, + medium: 32, + large: 64 +} + +// Shared render runtime for every generated icon. Extracting this from +// `createIconComponent` lets the generated icons ship as finished +// `React.forwardRef` components instead of runtime factory calls, while the +// size/`viewBox`/`closestNaturalHeight` math that depends on the runtime `size` +// prop stays here. +export function renderOcticon( + { + 'aria-label': ariaLabel, + 'aria-labelledby': arialabelledby, + tabIndex, + className = '', + fill = 'currentColor', + size = 16, + verticalAlign = 'text-bottom', + id, + title, + style, + ...rest + }, + forwardedRef, + defaultClassName, + svgDataByHeight, + heights +) { + const height = sizeMap[size] || size + const naturalHeight = closestNaturalHeight(heights, height) + const naturalWidth = svgDataByHeight[naturalHeight].width + const width = height * (naturalWidth / naturalHeight) + const path = svgDataByHeight[naturalHeight].path + const labelled = ariaLabel || arialabelledby + const role = labelled ? 'img' : undefined + + return ( + = 0 ? 'true' : 'false'} + aria-label={ariaLabel} + aria-labelledby={arialabelledby} + className={`${defaultClassName} ${className}`.trim()} + role={role} + viewBox={`0 0 ${naturalWidth} ${naturalHeight}`} + width={width} + height={height} + fill={fill} + id={id} + display="inline-block" + overflow="visible" + style={{ + verticalAlign, + ...style + }} + > + {title ? {title} : null} + {path} + + ) +} + +function closestNaturalHeight(naturalHeights, height) { + return naturalHeights + .map(naturalHeight => parseInt(naturalHeight, 10)) + .reduce((acc, naturalHeight) => (naturalHeight <= height ? naturalHeight : acc), naturalHeights[0]) +}