Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bundling): correct main field in package.json when using esbuild #12328

Merged
merged 1 commit into from Sep 29, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion e2e/esbuild/src/esbuild.test.ts
Expand Up @@ -13,7 +13,9 @@ import {

describe('EsBuild Plugin', () => {
let proj: string;

beforeEach(() => (proj = newProject()));

afterEach(() => cleanupProject());

it('should setup and build projects using build', async () => {
Expand All @@ -30,8 +32,10 @@ describe('EsBuild Plugin', () => {
runCLI(`build ${myPkg}`);

expect(runCommand(`node dist/libs/${myPkg}/index.js`)).toMatch(/Hello/);

// main field should be set correctly in package.json
checkFilesExist(`dist/libs/${myPkg}/package.json`);
expect(runCommand(`node dist/libs/${myPkg}`)).toMatch(/Hello/);

expect(readFile(`dist/libs/${myPkg}/assets/a.md`)).toMatch(/file a/);
expect(readFile(`dist/libs/${myPkg}/assets/b.md`)).toMatch(/file b/);

Expand Down
77 changes: 21 additions & 56 deletions packages/esbuild/src/executors/esbuild/esbuild.impl.ts
Expand Up @@ -15,8 +15,8 @@ import { normalizeOptions } from './lib/normalize';

import { EsBuildExecutorOptions } from './schema';
import { removeSync, writeJsonSync } from 'fs-extra';
import { getClientEnvironment } from '../../utils/environment-variables';
import { createAsyncIterable } from '@nrwl/js/src/utils/create-async-iterable/create-async-iteratable';
import { buildEsbuildOptions } from './lib/build-esbuild-options';

const ESM_FILE_EXTENSION = '.js';
const CJS_FILE_EXTENSION = '.cjs';
Expand Down Expand Up @@ -46,31 +46,19 @@ export async function* esbuildExecutor(
context
);

const esbuildOptions: esbuild.BuildOptions = {
...options.esbuildOptions,
entryPoints: [options.main, ...options.additionalEntryPoints],
entryNames:
options.outputHashing === 'all' ? '[dir]/[name].[hash]' : '[dir]/[name]',
outdir: options.outputPath,
bundle: true, // TODO(jack): support non-bundled builds
define: getClientEnvironment(),
external: options.external,
minify: options.minify,
platform: options.platform,
target: options.target,
metafile: options.metafile,
tsconfig: options.tsConfig,
};

if (options.watch) {
return yield* createAsyncIterable<{ success: boolean; outfile: string }>(
return yield* createAsyncIterable<{ success: boolean; outfile?: string }>(
async ({ next, done }) => {
let hasTypeErrors = false;

const results = await Promise.all(
options.format.map((format, idx) => {
const outfile = getOutfile(format, options, context);
return esbuild.build({
options.format.map(async (format, idx) => {
const esbuildOptions = buildEsbuildOptions(
format,
options,
context
);
const result = await esbuild.build({
...esbuildOptions,
watch:
// Only emit info on one of the watch processes.
Expand All @@ -95,15 +83,21 @@ export async function* esbuildExecutor(
logger.info(BUILD_WATCH_SUCCEEDED);
}

next({ success: !!error && !hasTypeErrors, outfile });
next({
success: !!error && !hasTypeErrors,
outfile: esbuildOptions.outfile,
});
},
}
: true,
format,
outExtension: {
'.js': getOutExtension(format),
},
});

next({
success,
outfile: esbuildOptions.outfile,
});

return result;
})
);
const processOnExit = () => {
Expand Down Expand Up @@ -133,23 +127,12 @@ export async function* esbuildExecutor(
} else {
logger.info(BUILD_WATCH_SUCCEEDED);
}

next({
success,
outfile: getOutfile(options.format[0], options, context),
});
}
);
} else {
const buildResults = await Promise.all(
options.format.map((format) =>
esbuild.build({
...esbuildOptions,
format,
outExtension: {
'.js': getOutExtension(format),
},
})
esbuild.build(buildEsbuildOptions(format, options, context))
)
);
const buildSuccess = buildResults.every((r) => r.errors?.length === 0);
Expand Down Expand Up @@ -200,24 +183,6 @@ function getTypeCheckOptions(
return typeCheckOptions;
}

function getOutExtension(format: 'cjs' | 'esm') {
return format === 'esm' ? ESM_FILE_EXTENSION : CJS_FILE_EXTENSION;
}

function getOutfile(
format: 'cjs' | 'esm',
options: EsBuildExecutorOptions,
context: ExecutorContext
) {
const candidate = joinPathFragments(
context.target.options.outputPath,
options.outputFileName
);
const ext = getOutExtension(format);
const { dir, name } = parse(candidate);
return `${dir}/${name}${ext}`;
}

async function runTypeCheck(
options: EsBuildExecutorOptions,
context: ExecutorContext
Expand Down
@@ -0,0 +1,154 @@
import { buildEsbuildOptions } from './build-esbuild-options';
import { ExecutorContext } from 'nx/src/config/misc-interfaces';

describe('buildEsbuildOptions', () => {
const context: ExecutorContext = {
workspace: {
version: 2,
projects: {
myapp: {
root: 'apps/myapp',
},
},
},
isVerbose: false,
root: '/',
cwd: '/',
target: {
executor: '@nrwl/esbuild:esbuild',
options: {
outputPath: 'dist/apps/myapp',
},
},
};

it('should include environment variables for platform === browser', () => {
expect(
buildEsbuildOptions(
'esm',
{
platform: 'browser',
main: 'apps/myapp/src/index.ts',
outputPath: 'dist/apps/myapp',
tsConfig: 'apps/myapp/tsconfig.app.json',
project: 'apps/myapp/package.json',
assets: [],
outputFileName: 'index.js',
singleEntry: true,
},
context
)
).toEqual({
bundle: true,
define: expect.objectContaining({
'process.env.NODE_ENV': '"test"',
}),
entryNames: '[dir]/[name]',
entryPoints: ['apps/myapp/src/index.ts'],
format: 'esm',
platform: 'browser',
outfile: 'dist/apps/myapp/index.js',
tsconfig: 'apps/myapp/tsconfig.app.json',
outExtension: {
'.js': '.js',
},
});
});

it('should support multiple entry points', () => {
expect(
buildEsbuildOptions(
'esm',
{
platform: 'browser',
main: 'apps/myapp/src/index.ts',
additionalEntryPoints: ['apps/myapp/src/extra-entry.ts'],
outputPath: 'dist/apps/myapp',
tsConfig: 'apps/myapp/tsconfig.app.json',
project: 'apps/myapp/package.json',
assets: [],
outputFileName: 'index.js',
singleEntry: false,
},
context
)
).toEqual({
bundle: true,
define: expect.objectContaining({
'process.env.NODE_ENV': '"test"',
}),
entryNames: '[dir]/[name]',
entryPoints: ['apps/myapp/src/index.ts', 'apps/myapp/src/extra-entry.ts'],
format: 'esm',
platform: 'browser',
outdir: 'dist/apps/myapp',
tsconfig: 'apps/myapp/tsconfig.app.json',
outExtension: {
'.js': '.js',
},
});
});

it('should support cjs format', () => {
expect(
buildEsbuildOptions(
'cjs',
{
platform: 'browser',
main: 'apps/myapp/src/index.ts',
outputPath: 'dist/apps/myapp',
tsConfig: 'apps/myapp/tsconfig.app.json',
project: 'apps/myapp/package.json',
assets: [],
outputFileName: 'index.js',
singleEntry: true,
},
context
)
).toEqual({
bundle: true,
define: expect.objectContaining({
'process.env.NODE_ENV': '"test"',
}),
entryNames: '[dir]/[name]',
entryPoints: ['apps/myapp/src/index.ts'],
format: 'cjs',
platform: 'browser',
outfile: 'dist/apps/myapp/index.cjs',
tsconfig: 'apps/myapp/tsconfig.app.json',
outExtension: {
'.js': '.cjs',
},
});
});

it('should not define environment variables for node', () => {
expect(
buildEsbuildOptions(
'cjs',
{
platform: 'node',
main: 'apps/myapp/src/index.ts',
outputPath: 'dist/apps/myapp',
tsConfig: 'apps/myapp/tsconfig.app.json',
project: 'apps/myapp/package.json',
assets: [],
outputFileName: 'index.js',
singleEntry: true,
},
context
)
).toEqual({
bundle: true,
entryNames: '[dir]/[name]',
entryPoints: ['apps/myapp/src/index.ts'],
format: 'cjs',
platform: 'node',
outfile: 'dist/apps/myapp/index.cjs',
tsconfig: 'apps/myapp/tsconfig.app.json',
outExtension: {
'.js': '.cjs',
},
});
});
});
@@ -0,0 +1,66 @@
import * as esbuild from 'esbuild';
import { getClientEnvironment } from '../../../utils/environment-variables';
import {
EsBuildExecutorOptions,
NormalizedEsBuildExecutorOptions,
} from '../schema';
import { ExecutorContext } from 'nx/src/config/misc-interfaces';
import { joinPathFragments } from 'nx/src/utils/path';
import { parse } from 'path';

const ESM_FILE_EXTENSION = '.js';
const CJS_FILE_EXTENSION = '.cjs';

export function buildEsbuildOptions(
format: 'cjs' | 'esm',
options: NormalizedEsBuildExecutorOptions,
context: ExecutorContext
): esbuild.BuildOptions {
const esbuildOptions: esbuild.BuildOptions = {
...options.esbuildOptions,
entryPoints: options.additionalEntryPoints
? [options.main, ...options.additionalEntryPoints]
: [options.main],
entryNames:
options.outputHashing === 'all' ? '[dir]/[name].[hash]' : '[dir]/[name]',
bundle: true, // TODO(jack): support non-bundled builds
external: options.external,
minify: options.minify,
platform: options.platform,
target: options.target,
metafile: options.metafile,
tsconfig: options.tsConfig,
format,
outExtension: { '.js': getOutExtension(format) },
};

if (options.platform === 'browser') {
esbuildOptions.define = getClientEnvironment();
}

if (options.singleEntry) {
esbuildOptions.outfile = getOutfile(format, options, context);
} else {
esbuildOptions.outdir = options.outputPath;
}

return esbuildOptions;
}

function getOutExtension(format: 'cjs' | 'esm') {
return format === 'esm' ? ESM_FILE_EXTENSION : CJS_FILE_EXTENSION;
}

function getOutfile(
format: 'cjs' | 'esm',
options: EsBuildExecutorOptions,
context: ExecutorContext
) {
const candidate = joinPathFragments(
context.target.options.outputPath,
options.outputFileName
);
const ext = getOutExtension(format);
const { dir, name } = parse(candidate);
return `${dir}/${name}${ext}`;
}