Skip to content

Commit

Permalink
fix: enable esm format (#342)
Browse files Browse the repository at this point in the history
  • Loading branch information
samchungy committed Jul 20, 2022
1 parent 54a25bd commit fd3d4e6
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 40 deletions.
12 changes: 5 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ See [example folder](examples) for some example configurations.
| `nativeZip` | Uses the system's `zip` executable to create archives. _NOTE_: This will produce non-deterministic archives which causes a Serverless deployment update on every deploy. | `false` |
| `outputBuildFolder` | The output folder for Esbuild builds within the work folder. | `'.build'` |
| `outputWorkFolder` | The output folder for this plugin where all the bundle preparation is done. | `'.esbuild'` |
| `outputFileExtension` | The file extension used for the bundled output file. This will override the esbuild `outExtension` option | `'.js'` |
| `outputFileExtension` | The file extension used for the bundled output file. This will override the esbuild `outExtension` option | `'.js'` |
| `packagePath` | Path to the `package.json` file for `external` dependency resolution. | `'./package.json'` |
| `packager` | Packager to use for `external` dependency resolution. Values: `npm`, `yarn`, `pnpm` | `'npm'` |
| `packagerOptions` | Extra options for packagers for `external` dependency resolution. | [Packager Options](#packager-options) |
Expand All @@ -99,7 +99,7 @@ The following `esbuild` options are automatically set.
| `entryPoints` | N/A | Cannot be overridden |
| `incremental` | N/A | Cannot be overridden. Use `disableIncremental` to disable it |
| `outDir` | N/A | Cannot be overridden |
| `platform` | `'node'` | Set to `'neutral'` to enable ESM support |
| `platform` | `'node'` | Set `format` to `esm` to enable ESM support |
| `target` | `'node12'` | We dynamically set this. See [Supported Runtimes](#supported-runtimes) |
| `watch` | N/A | Cannot be overridden |

Expand Down Expand Up @@ -147,11 +147,9 @@ custom:
```js
// esbuild.config.js
module.exports = (serverless) => ({
external: [
'lodash'
],
plugins: []
})
external: ['lodash'],
plugins: [],
});
```

### Including Extra Files
Expand Down
12 changes: 5 additions & 7 deletions src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import path from 'path';
import { uniq } from 'ramda';

import EsbuildServerlessPlugin from '.';
import { isESM } from './helper';
import { FileBuildResult } from './types';
import { trimExtension } from './utils';

Expand All @@ -27,22 +28,19 @@ export async function bundle(this: EsbuildServerlessPlugin, incremental = false)
plugins: this.plugins,
};

if (
this.buildOptions.platform === 'neutral' &&
this.buildOptions.outputFileExtension === '.cjs'
) {
if (isESM(this.buildOptions) && this.buildOptions.outputFileExtension === '.cjs') {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore Serverless typings (as of v3.0.2) are incorrect
throw new this.serverless.classes.Error(
'ERROR: platform "neutral" should not output a file with extension ".cjs".'
'ERROR: format "esm" or platform "neutral" should not output a file with extension ".cjs".'
);
}

if (this.buildOptions.platform === 'node' && this.buildOptions.outputFileExtension === '.mjs') {
if (!isESM(this.buildOptions) && this.buildOptions.outputFileExtension === '.mjs') {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore Serverless typings (as of v3.0.2) are incorrect
throw new this.serverless.classes.Error(
'ERROR: platform "node" should not output a file with extension ".mjs".'
'ERROR: Non esm builds should not output a file with extension ".mjs".'
);
}

Expand Down
23 changes: 13 additions & 10 deletions src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { uniq } from 'ramda';
import Serverless from 'serverless';
import ServerlessPlugin from 'serverless/classes/Plugin';
import matchAll from 'string.prototype.matchall';
import { DependencyMap, FunctionEntry } from './types';
import { Configuration, DependencyMap, FunctionEntry } from './types';

export function extractFunctionEntries(
cwd: string,
Expand Down Expand Up @@ -116,20 +116,23 @@ export const flatDep = (root: DependencyMap, rootDepsFilter: string[]): string[]
*/
const getBaseDep = (path: string): string => /^@[^/]+\/[^/\n]+|^[^/\n]+/.exec(path)[0];

export const isESM = (buildOptions: Configuration): boolean => {
return (
buildOptions.format === 'esm' || (buildOptions.platform === 'neutral' && !buildOptions.format)
);
};

/**
* Extracts the list of dependencies that appear in a bundle as `require(XXX)`
* @param bundlePath Absolute path to a bundled JS file
*/
export const getDepsFromBundle = (bundlePath: string, platform: string): string[] => {
export const getDepsFromBundle = (bundlePath: string, useESM: boolean): string[] => {
const bundleContent = fs.readFileSync(bundlePath, 'utf8');
const deps =
platform === 'neutral'
? Array.from<string>(matchAll(bundleContent, /from\s?"(.*?)";|import\s?"(.*?)";/gim)).map(
(match) => match[1] || match[2]
)
: Array.from<string>(matchAll(bundleContent, /require\("(.*?)"\)/gim)).map(
(match) => match[1]
);
const deps = useESM
? Array.from<string>(matchAll(bundleContent, /from\s?"(.*?)";|import\s?"(.*?)";/gim)).map(
(match) => match[1] || match[2]
)
: Array.from<string>(matchAll(bundleContent, /require\("(.*?)"\)/gim)).map((match) => match[1]);
return uniq(deps.map((dep): string => getBaseDep(dep)));
};

Expand Down
4 changes: 2 additions & 2 deletions src/pack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import semver from 'semver';
import EsbuildServerlessPlugin from '.';
import { ONLY_PREFIX, SERVERLESS_FOLDER } from './constants';
import { doSharePath, flatDep, getDepsFromBundle } from './helper';
import { doSharePath, flatDep, getDepsFromBundle, isESM } from './helper';
import * as Packagers from './packagers';
import { IFiles } from './types';
import { humanSize, zip, trimExtension } from './utils';
Expand Down Expand Up @@ -176,7 +176,7 @@ export async function pack(this: EsbuildServerlessPlugin) {
if (hasExternals) {
const bundleDeps = getDepsFromBundle(
path.join(this.buildDirPath, bundlePath),
this.buildOptions.platform
isESM(this.buildOptions)
);
const bundleExternals = intersection(bundleDeps, externals);
depWhiteList = flatDep(packagerDependenciesList.dependencies, bundleExternals);
Expand Down
4 changes: 2 additions & 2 deletions src/tests/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ describe('buildOption platform node', () => {

const plugin = esbuildPlugin({ functionEntries, buildOptions: buildOptions as any });

const expectedError = 'ERROR: platform "node" should not output a file with extension ".mjs".';
const expectedError = 'ERROR: Non esm builds should not output a file with extension ".mjs".';

try {
await bundle.call(plugin);
Expand Down Expand Up @@ -496,7 +496,7 @@ describe('buildOption platform neutral', () => {
const plugin = esbuildPlugin({ functionEntries, buildOptions: buildOptions as any });

const expectedError =
'ERROR: platform "neutral" should not output a file with extension ".cjs".';
'ERROR: format "esm" or platform "neutral" should not output a file with extension ".cjs".';

try {
await bundle.call(plugin);
Expand Down
47 changes: 35 additions & 12 deletions src/tests/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import path from 'path';
import os from 'os';
import { mocked } from 'ts-jest/utils';

import { extractFunctionEntries, flatDep, getDepsFromBundle } from '../helper';
import { DependencyMap } from '../types';
import { extractFunctionEntries, flatDep, getDepsFromBundle, isESM } from '../helper';
import { Configuration, DependencyMap } from '../types';

jest.mock('fs-extra');

Expand Down Expand Up @@ -148,18 +148,17 @@ describe('extractFunctionEntries', () => {

describe('getDepsFromBundle', () => {
const path = './';
describe('node platform', () => {
const platform = 'node';
describe('useESM = false', () => {
it('should extract deps from a string', () => {
mocked(fs).readFileSync.mockReturnValue('require("@scope/package1");require("package2")');
expect(getDepsFromBundle(path, platform)).toStrictEqual(['@scope/package1', 'package2']);
expect(getDepsFromBundle(path, false)).toStrictEqual(['@scope/package1', 'package2']);
});

it('should extract the base dep from a string', () => {
mocked(fs).readFileSync.mockReturnValue(
'require("@scope/package1/subpath");require("package2/subpath");require("@scope/package3/subpath/subpath")require("package4/subpath/subpath")'
);
expect(getDepsFromBundle(path, platform)).toStrictEqual([
expect(getDepsFromBundle(path, false)).toStrictEqual([
'@scope/package1',
'package2',
'@scope/package3',
Expand All @@ -171,13 +170,11 @@ describe('getDepsFromBundle', () => {
mocked(fs).readFileSync.mockReturnValue(
'require("package1/subpath");require("package1");require("package1")'
);
expect(getDepsFromBundle(path, platform)).toStrictEqual(['package1']);
expect(getDepsFromBundle(path, false)).toStrictEqual(['package1']);
});
});

describe('neutral platform', () => {
const platform = 'neutral';

describe('useESM = true', () => {
it('should extract deps from a string', () => {
mocked(fs).readFileSync.mockReturnValue(
`
Expand All @@ -186,17 +183,43 @@ describe('getDepsFromBundle', () => {
import {hello as r} from "package3";
`
);
expect(getDepsFromBundle(path, platform)).toStrictEqual(['package1', 'package2', 'package3']);
expect(getDepsFromBundle(path, true)).toStrictEqual(['package1', 'package2', 'package3']);
});
it('should extract deps from a minified string', () => {
mocked(fs).readFileSync.mockReturnValue(
'import*as n from"package1";import"package2";import{hello as r}from"package3";'
);
expect(getDepsFromBundle(path, platform)).toStrictEqual(['package1', 'package2', 'package3']);
expect(getDepsFromBundle(path, true)).toStrictEqual(['package1', 'package2', 'package3']);
});
});
});

describe('isESM', () => {
it('should return true when format is set to esm', () => {
const config = {
format: 'esm',
} as Partial<Configuration> as Configuration;

expect(isESM(config)).toBe(true);
});

it('should return true when platform is set to neutral and format is not set', () => {
const config = {
platform: 'neutral',
} as Partial<Configuration> as Configuration;

expect(isESM(config)).toBe(true);
});

it('should return false when platform is set to node and format is not set', () => {
const config = {
platform: 'node',
} as Partial<Configuration> as Configuration;

expect(isESM(config)).toBe(false);
});
});

describe('flatDeps', () => {
describe('basic', () => {
it('should pull all the dependencies from samchungy-a and samchungy-b', () => {
Expand Down

0 comments on commit fd3d4e6

Please sign in to comment.