Skip to content

Commit

Permalink
build(@deephaven/icons): Properly package icons and remove unnecessar…
Browse files Browse the repository at this point in the history
…y files in dist (#1437)

While updating packages consumed in the docs, I noticed our icons
`package.json` was off. It specified it was a module type (ESM), but
exported CJS as `.js`. This is incorrect as those will be interpreted as
ESM by Node. Instead we should be exporting a `.cjs` file and a `.js` or
`.mjs` as the main file.

Added an exports section to the `package.json` to indicate which file is
for ESM and which is for CJS. We still need to distribute CJS for Jest
tests.

Removed unnecessary files from the distribution. We don't use imports
like `import vsTrash from '@deephaven/icons/vsTrash';` and the ESM tree
shakes, so I removed the individual files exported. They also would have
been imported from `/icons/dist` which is bad practice. Removed the SVG
files from the files for the published package as well since they aren't
used by anything in production.

BREAKING CHANGE: Any imports/aliasing to `@deephaven/icons/dist` should
be removed and just read the package contents normally (e.g. DHE jest
and vite configs for using community packages locally). See the changes
to vite and jest configs in this change for how to update
  • Loading branch information
mattrunyon authored Aug 4, 2023
1 parent 6e0b60e commit ec7ccef
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 126 deletions.
13 changes: 4 additions & 9 deletions jest.config.base.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ module.exports = {
'./__mocks__/spectrumTheme$1Mock.js'
),
'\\.(css|less|scss|sass)$': 'identity-obj-proxy',
'\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': path.join(
__dirname,
'./__mocks__/fileMock.js'
),
'\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
path.join(__dirname, './__mocks__/fileMock.js'),
'^fira$': 'identity-obj-proxy',
'^monaco-editor$': path.join(
__dirname,
Expand All @@ -26,11 +24,8 @@ module.exports = {
),
// Handle monaco worker files
'\\.worker.*$': 'identity-obj-proxy',
'^@deephaven/icons$': path.join(
__dirname,
'./packages/icons/dist/index.js'
),
'^@deephaven/(.*)$': path.join(__dirname, './packages/$1/src'),
// All packages except icons use src code
'^@deephaven/(?!icons)(.*)$': path.join(__dirname, './packages/$1/src'),
},
testEnvironment: 'jsdom',
setupFilesAfterEnv: [path.join(__dirname, './jest.setup.ts')],
Expand Down
6 changes: 1 addition & 5 deletions packages/code-studio/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ export default defineConfig(({ mode }) => {
replacement: `${packagesDir}/$1/scss/$2`,
},
{
find: /^@deephaven\/icons$/,
replacement: `${packagesDir}/icons/dist/index.es.js`,
},
{
find: /^@deephaven\/(.*)/,
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
replacement: `${packagesDir}/$1/src`,
},
]
Expand Down
6 changes: 1 addition & 5 deletions packages/embed-chart/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,7 @@ export default defineConfig(({ mode }) => {
replacement: `${packagesDir}/$1/scss/$2`,
},
{
find: /^@deephaven\/icons$/,
replacement: `${packagesDir}/icons/dist/index.es.js`,
},
{
find: /^@deephaven\/(.*)/,
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
replacement: `${packagesDir}/$1/src`,
},
]
Expand Down
6 changes: 1 addition & 5 deletions packages/embed-grid/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,7 @@ export default defineConfig(({ mode }) => {
replacement: `${packagesDir}/$1/scss/$2`,
},
{
find: /^@deephaven\/icons$/,
replacement: `${packagesDir}/icons/dist/index.es.js`,
},
{
find: /^@deephaven\/(.*)/,
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
replacement: `${packagesDir}/$1/src`,
},
]
Expand Down
18 changes: 13 additions & 5 deletions packages/icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
"version": "0.45.0",
"description": "Icons used in Deephaven client apps. Extends vscode-codicons to be font-awesome svg-core compatible and adds additional icons in a similar style.",
"main": "dist/index.js",
"module": "dist/index.es.js",
"type": "module",
"types": "dist/index.d.ts",
"type": "module",
"exports": {
".": {
"require": "./dist/index.cjs",
"import": "./dist/index.js"
}
},
"files": [
"dist"
"dist/index.cjs",
"dist/index.js",
"dist/index.d.ts"
],
"sideEffects": false,
"scripts": {
Expand All @@ -16,9 +23,10 @@
"watch": "chokidar \"src/**/*\" -c \"npm run build\"",
"svgo-dh": "svgo -q -f ./src/icons/ -o ./dist/svg/dh",
"svgo-vs": "svgo -q -f \"../../node_modules/@vscode/codicons/src/icons/\" -o ./dist/svg/vs",
"build": "run-s build:icons build:js",
"build": "run-s build:icons build:js clean:dist:icons",
"build:icons": "run-p svgo-dh svgo-vs",
"build:js": "node ./scripts/build.js"
"build:js": "node ./scripts/build.js",
"clean:dist:icons": "rimraf ./dist/svg"
},
"repository": {
"type": "git",
Expand Down
41 changes: 8 additions & 33 deletions packages/icons/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import path from 'path';
import parser from 'svg-parser';
import svgPathTools from 'svg-path-tools';
// template shape based on fortawesome/fontawesome-free export file shape
import { dtsFile, jsFile } from './template/file.js';
import { indexdts, indexjs, indexesjs } from './template/indicies.js';
import { dtsIndex, cjsIndex, mjsIndex } from './template/indicies.js';

const { parse } = parser;
const { parse: parsePath, scale, stringify } = svgPathTools;
Expand Down Expand Up @@ -76,27 +75,6 @@ function parseSvg(content) {
}
}

async function createDefinition(file) {
// use imported template literals to build files
const dtsContent = dtsFile(file.prefixedName);
const jsContent = jsFile(file, file.width, file.height, file.path);

try {
await Promise.all([
await fs.writeFile(
path.join(BUILD_DIR, `${file.prefixedName}.d.ts`),
dtsContent
),
await fs.writeFile(
path.join(BUILD_DIR, `${file.prefixedName}.js`),
jsContent
),
]);
} catch (e) {
console.error('Unable to create definition for', file, ':', e);
}
}

async function getSVGFiles(src) {
const files = await getFiles(src);
return files.map(f => {
Expand Down Expand Up @@ -200,20 +178,17 @@ async function build(buildSources) {
// flatten the array, as mulitple sources may have been passed in
files = files.flat();

// write out the individual definition files
await Promise.all(files.map(file => createDefinition(file)));

// write out an index.d.ts
const indexdtsContent = indexdts(files, buildSources);
const indexdtsContent = dtsIndex(files, buildSources);
await fs.writeFile(`${BUILD_DIR}/index.d.ts`, indexdtsContent);

// write out index.js
const indexjsContent = indexjs(files);
await fs.writeFile(`${BUILD_DIR}/index.js`, indexjsContent);
// write out CJS
const indexjsContent = cjsIndex(files);
await fs.writeFile(`${BUILD_DIR}/index.cjs`, indexjsContent);

// write out index.es.js
const indexesjsContent = indexesjs(files);
await fs.writeFile(`${BUILD_DIR}/index.es.js`, indexesjsContent);
// write out ESM
const indexesjsContent = mjsIndex(files);
await fs.writeFile(`${BUILD_DIR}/index.js`, indexesjsContent);

console.log('deephaven-app-icons build complete!');
}
Expand Down
46 changes: 0 additions & 46 deletions packages/icons/scripts/template/file.js

This file was deleted.

32 changes: 14 additions & 18 deletions packages/icons/scripts/template/indicies.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export const indexdts = (files, sources) => {
export const dtsIndex = (files, sources) => {
const top = files
.map(file => `export const ${file.prefixedName}: IconDefinition;`)
.join('\n');
Expand All @@ -7,25 +7,23 @@ export const indexdts = (files, sources) => {
.map(src => `export const ${src.prefix}: IconPack;`)
.join('\n');

return `
${top}
return `${top}
import { IconDefinition, IconLookup, IconName, IconPrefix, IconPack } from '@fortawesome/fontawesome-common-types';
export { IconDefinition, IconLookup, IconName, IconPrefix, IconPack } from '@fortawesome/fontawesome-common-types';
export const prefix: IconPrefix;
${bottom}
`;
};

export const indexjs = files => {
export const cjsIndex = files => {
const iconVar = files
.map(
file => `
var ${file.prefixedName} = {
file => `const ${file.prefixedName} = {
prefix: "${file.prefix}",
iconName: "${file.name}",
icon: [${file.width}, ${file.height}, [], "f000", ${
file.path ? `"${file.path}"` : file.path
}]
file.path ? `"${file.path}"` : file.path
}]
};`
)
.join('');
Expand All @@ -44,8 +42,8 @@ export const indexjs = files => {
(factory((global['deephaven-app-icons'] = {})));
}(this, (function (exports) { 'use strict';
var prefix = "dh";${iconVar}
var _iconsCache = {
const prefix = "dh";${iconVar}
const _iconsCache = {
${cache}
};
Expand All @@ -59,16 +57,15 @@ ${exps}
`;
};

export const indexesjs = files => {
export const mjsIndex = files => {
const iconVar = files
.map(
file => `
var ${file.prefixedName} = {
file => `const ${file.prefixedName} = {
prefix: "${file.prefix}",
iconName: "${file.name}",
icon: [${file.width}, ${file.height}, [], "f000", ${
file.path ? `"${file.path}"` : file.path
}]
file.path ? `"${file.path}"` : file.path
}]
};`
)
.join('');
Expand All @@ -79,9 +76,8 @@ var ${file.prefixedName} = {

const exps = files.map(file => `${file.prefixedName}, `).join('');

return `
var prefix = "dh";${iconVar}
var _iconsCache = {
return `const prefix = "dh";${iconVar}
const _iconsCache = {
${cache}
};
Expand Down

0 comments on commit ec7ccef

Please sign in to comment.