Skip to content

Commit

Permalink
fix: mark polyfills as side-effect free (#141)
Browse files Browse the repository at this point in the history
  • Loading branch information
markdalgleish authored Aug 14, 2023
1 parent 08f9f17 commit 30eda0a
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/lib/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'node:path';
import { getCachedPolyfillContent, getCachedPolyfillPath } from './polyfill.js';
import { escapeRegex, commonJsTemplate, normalizeNodeBuiltinPath } from './utils/util.js';

import type { OnResolveArgs, Plugin } from 'esbuild';
import type { OnResolveArgs, OnResolveResult, Plugin } from 'esbuild';
import type esbuild from 'esbuild';

const NAME = 'node-modules-polyfills';
Expand Down Expand Up @@ -108,7 +108,7 @@ export const nodeModulesPolyfillPlugin = (options: NodePolyfillsOptions = {}): P
.join('|')})$`,
);

const resolver = async (args: OnResolveArgs) => {
const resolver = async (args: OnResolveArgs): Promise<OnResolveResult | undefined> => {
const moduleName = normalizeNodeBuiltinPath(args.path);

if (!modules[moduleName]) {
Expand All @@ -119,6 +119,7 @@ export const nodeModulesPolyfillPlugin = (options: NodePolyfillsOptions = {}): P
return {
namespace: emptyNamespace,
path: args.path,
sideEffects: false,
};
}

Expand All @@ -134,6 +135,7 @@ export const nodeModulesPolyfillPlugin = (options: NodePolyfillsOptions = {}): P
return {
namespace: isCommonjs ? commonjsNamespace : namespace,
path: args.path,
sideEffects: false,
};
};

Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures/input/unusedImport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { message } from './unusedImport/exports';

console.log(message);
3 changes: 3 additions & 0 deletions tests/fixtures/input/unusedImport/exports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const message = 'Hello world';

export { default as assertStrict } from 'node:assert/strict';
10 changes: 10 additions & 0 deletions tests/scenarios/__snapshots__/unusedImport.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`Unused Import Test > GIVEN a file that imports from a module exporting a node builtin and does not use it THEN it should be removed by esbuild 1`] = `
"// tests/fixtures/input/unusedImport/exports.ts
var message = \\"Hello world\\";
// tests/fixtures/input/unusedImport.ts
console.log(message);
"
`;
25 changes: 25 additions & 0 deletions tests/scenarios/unusedImport.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import esbuild, { type BuildOptions } from 'esbuild';

import { assertFileContent, buildAbsolutePath, createEsbuildConfig } from '../util';

import type { NodePolyfillsOptions } from '../../dist';

function createConfig(pluginOptions?: NodePolyfillsOptions): BuildOptions {
return createEsbuildConfig(
{
format: 'esm',
entryPoints: [buildAbsolutePath('./fixtures/input/unusedImport.ts')],
},
pluginOptions,
);
}

describe('Unused Import Test', () => {
test('GIVEN a file that imports from a module exporting a node builtin and does not use it THEN it should be removed by esbuild', async () => {
const config = createConfig();

await esbuild.build(config);

await assertFileContent('./fixtures/output/unusedImport.js');
});
});

0 comments on commit 30eda0a

Please sign in to comment.