Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/octicons-react-codesplitting.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 12 additions & 0 deletions lib/octicons_react/__tests__/__fixtures__/dynamic-imports.mjs
Original file line number Diff line number Diff line change
@@ -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
}
}
50 changes: 50 additions & 0 deletions lib/octicons_react/__tests__/codesplitting.test.js
Original file line number Diff line number Diff line change
@@ -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')
})
2 changes: 1 addition & 1 deletion lib/octicons_react/__tests__/tree-shaking.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"`)
})
16 changes: 11 additions & 5 deletions lib/octicons_react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
67 changes: 43 additions & 24 deletions lib/octicons_react/rollup.config.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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')
Comment thread
mattcosta7 marked this conversation as resolved.

// 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',
Expand Down
144 changes: 97 additions & 47 deletions lib/octicons_react/script/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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. */'

Expand All @@ -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 = {
Expand All @@ -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: <path d="..." /> } }
// 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 <svg> 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'
Expand All @@ -108,23 +140,41 @@ interface IconProps {

type Icon = React.FC<IconProps>

${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)
Expand Down
Loading
Loading