Skip to content

Commit ec7ccef

Browse files
authored
build(@deephaven/icons): Properly package icons and remove unnecessary 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
1 parent 6e0b60e commit ec7ccef

8 files changed

Lines changed: 42 additions & 126 deletions

File tree

jest.config.base.cjs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@ module.exports = {
1414
'./__mocks__/spectrumTheme$1Mock.js'
1515
),
1616
'\\.(css|less|scss|sass)$': 'identity-obj-proxy',
17-
'\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': path.join(
18-
__dirname,
19-
'./__mocks__/fileMock.js'
20-
),
17+
'\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
18+
path.join(__dirname, './__mocks__/fileMock.js'),
2119
'^fira$': 'identity-obj-proxy',
2220
'^monaco-editor$': path.join(
2321
__dirname,
@@ -26,11 +24,8 @@ module.exports = {
2624
),
2725
// Handle monaco worker files
2826
'\\.worker.*$': 'identity-obj-proxy',
29-
'^@deephaven/icons$': path.join(
30-
__dirname,
31-
'./packages/icons/dist/index.js'
32-
),
33-
'^@deephaven/(.*)$': path.join(__dirname, './packages/$1/src'),
27+
// All packages except icons use src code
28+
'^@deephaven/(?!icons)(.*)$': path.join(__dirname, './packages/$1/src'),
3429
},
3530
testEnvironment: 'jsdom',
3631
setupFilesAfterEnv: [path.join(__dirname, './jest.setup.ts')],

packages/code-studio/vite.config.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,7 @@ export default defineConfig(({ mode }) => {
8787
replacement: `${packagesDir}/$1/scss/$2`,
8888
},
8989
{
90-
find: /^@deephaven\/icons$/,
91-
replacement: `${packagesDir}/icons/dist/index.es.js`,
92-
},
93-
{
94-
find: /^@deephaven\/(.*)/,
90+
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
9591
replacement: `${packagesDir}/$1/src`,
9692
},
9793
]

packages/embed-chart/vite.config.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,7 @@ export default defineConfig(({ mode }) => {
6464
replacement: `${packagesDir}/$1/scss/$2`,
6565
},
6666
{
67-
find: /^@deephaven\/icons$/,
68-
replacement: `${packagesDir}/icons/dist/index.es.js`,
69-
},
70-
{
71-
find: /^@deephaven\/(.*)/,
67+
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
7268
replacement: `${packagesDir}/$1/src`,
7369
},
7470
]

packages/embed-grid/vite.config.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,7 @@ export default defineConfig(({ mode }) => {
6464
replacement: `${packagesDir}/$1/scss/$2`,
6565
},
6666
{
67-
find: /^@deephaven\/icons$/,
68-
replacement: `${packagesDir}/icons/dist/index.es.js`,
69-
},
70-
{
71-
find: /^@deephaven\/(.*)/,
67+
find: /^@deephaven\/(?!icons)(.*)/, // Icons package can not import from src
7268
replacement: `${packagesDir}/$1/src`,
7369
},
7470
]

packages/icons/package.json

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@
33
"version": "0.45.0",
44
"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.",
55
"main": "dist/index.js",
6-
"module": "dist/index.es.js",
7-
"type": "module",
86
"types": "dist/index.d.ts",
7+
"type": "module",
8+
"exports": {
9+
".": {
10+
"require": "./dist/index.cjs",
11+
"import": "./dist/index.js"
12+
}
13+
},
914
"files": [
10-
"dist"
15+
"dist/index.cjs",
16+
"dist/index.js",
17+
"dist/index.d.ts"
1118
],
1219
"sideEffects": false,
1320
"scripts": {
@@ -16,9 +23,10 @@
1623
"watch": "chokidar \"src/**/*\" -c \"npm run build\"",
1724
"svgo-dh": "svgo -q -f ./src/icons/ -o ./dist/svg/dh",
1825
"svgo-vs": "svgo -q -f \"../../node_modules/@vscode/codicons/src/icons/\" -o ./dist/svg/vs",
19-
"build": "run-s build:icons build:js",
26+
"build": "run-s build:icons build:js clean:dist:icons",
2027
"build:icons": "run-p svgo-dh svgo-vs",
21-
"build:js": "node ./scripts/build.js"
28+
"build:js": "node ./scripts/build.js",
29+
"clean:dist:icons": "rimraf ./dist/svg"
2230
},
2331
"repository": {
2432
"type": "git",

packages/icons/scripts/build.js

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import path from 'path';
55
import parser from 'svg-parser';
66
import svgPathTools from 'svg-path-tools';
77
// template shape based on fortawesome/fontawesome-free export file shape
8-
import { dtsFile, jsFile } from './template/file.js';
9-
import { indexdts, indexjs, indexesjs } from './template/indicies.js';
8+
import { dtsIndex, cjsIndex, mjsIndex } from './template/indicies.js';
109

1110
const { parse } = parser;
1211
const { parse: parsePath, scale, stringify } = svgPathTools;
@@ -76,27 +75,6 @@ function parseSvg(content) {
7675
}
7776
}
7877

79-
async function createDefinition(file) {
80-
// use imported template literals to build files
81-
const dtsContent = dtsFile(file.prefixedName);
82-
const jsContent = jsFile(file, file.width, file.height, file.path);
83-
84-
try {
85-
await Promise.all([
86-
await fs.writeFile(
87-
path.join(BUILD_DIR, `${file.prefixedName}.d.ts`),
88-
dtsContent
89-
),
90-
await fs.writeFile(
91-
path.join(BUILD_DIR, `${file.prefixedName}.js`),
92-
jsContent
93-
),
94-
]);
95-
} catch (e) {
96-
console.error('Unable to create definition for', file, ':', e);
97-
}
98-
}
99-
10078
async function getSVGFiles(src) {
10179
const files = await getFiles(src);
10280
return files.map(f => {
@@ -200,20 +178,17 @@ async function build(buildSources) {
200178
// flatten the array, as mulitple sources may have been passed in
201179
files = files.flat();
202180

203-
// write out the individual definition files
204-
await Promise.all(files.map(file => createDefinition(file)));
205-
206181
// write out an index.d.ts
207-
const indexdtsContent = indexdts(files, buildSources);
182+
const indexdtsContent = dtsIndex(files, buildSources);
208183
await fs.writeFile(`${BUILD_DIR}/index.d.ts`, indexdtsContent);
209184

210-
// write out index.js
211-
const indexjsContent = indexjs(files);
212-
await fs.writeFile(`${BUILD_DIR}/index.js`, indexjsContent);
185+
// write out CJS
186+
const indexjsContent = cjsIndex(files);
187+
await fs.writeFile(`${BUILD_DIR}/index.cjs`, indexjsContent);
213188

214-
// write out index.es.js
215-
const indexesjsContent = indexesjs(files);
216-
await fs.writeFile(`${BUILD_DIR}/index.es.js`, indexesjsContent);
189+
// write out ESM
190+
const indexesjsContent = mjsIndex(files);
191+
await fs.writeFile(`${BUILD_DIR}/index.js`, indexesjsContent);
217192

218193
console.log('deephaven-app-icons build complete!');
219194
}

packages/icons/scripts/template/file.js

Lines changed: 0 additions & 46 deletions
This file was deleted.

packages/icons/scripts/template/indicies.js

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export const indexdts = (files, sources) => {
1+
export const dtsIndex = (files, sources) => {
22
const top = files
33
.map(file => `export const ${file.prefixedName}: IconDefinition;`)
44
.join('\n');
@@ -7,25 +7,23 @@ export const indexdts = (files, sources) => {
77
.map(src => `export const ${src.prefix}: IconPack;`)
88
.join('\n');
99

10-
return `
11-
${top}
10+
return `${top}
1211
import { IconDefinition, IconLookup, IconName, IconPrefix, IconPack } from '@fortawesome/fontawesome-common-types';
1312
export { IconDefinition, IconLookup, IconName, IconPrefix, IconPack } from '@fortawesome/fontawesome-common-types';
1413
export const prefix: IconPrefix;
1514
${bottom}
1615
`;
1716
};
1817

19-
export const indexjs = files => {
18+
export const cjsIndex = files => {
2019
const iconVar = files
2120
.map(
22-
file => `
23-
var ${file.prefixedName} = {
21+
file => `const ${file.prefixedName} = {
2422
prefix: "${file.prefix}",
2523
iconName: "${file.name}",
2624
icon: [${file.width}, ${file.height}, [], "f000", ${
27-
file.path ? `"${file.path}"` : file.path
28-
}]
25+
file.path ? `"${file.path}"` : file.path
26+
}]
2927
};`
3028
)
3129
.join('');
@@ -44,8 +42,8 @@ export const indexjs = files => {
4442
(factory((global['deephaven-app-icons'] = {})));
4543
}(this, (function (exports) { 'use strict';
4644
47-
var prefix = "dh";${iconVar}
48-
var _iconsCache = {
45+
const prefix = "dh";${iconVar}
46+
const _iconsCache = {
4947
${cache}
5048
};
5149
@@ -59,16 +57,15 @@ ${exps}
5957
`;
6058
};
6159

62-
export const indexesjs = files => {
60+
export const mjsIndex = files => {
6361
const iconVar = files
6462
.map(
65-
file => `
66-
var ${file.prefixedName} = {
63+
file => `const ${file.prefixedName} = {
6764
prefix: "${file.prefix}",
6865
iconName: "${file.name}",
6966
icon: [${file.width}, ${file.height}, [], "f000", ${
70-
file.path ? `"${file.path}"` : file.path
71-
}]
67+
file.path ? `"${file.path}"` : file.path
68+
}]
7269
};`
7370
)
7471
.join('');
@@ -79,9 +76,8 @@ var ${file.prefixedName} = {
7976

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

82-
return `
83-
var prefix = "dh";${iconVar}
84-
var _iconsCache = {
79+
return `const prefix = "dh";${iconVar}
80+
const _iconsCache = {
8581
${cache}
8682
};
8783

0 commit comments

Comments
 (0)