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.

Sign up for GitHub

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

Jump to bottom

feat(react): provide only umd & esm bundles when packaging #2524

Merged
merged 2 commits into from
Feb 24, 2020
Merged

feat(react): provide only umd & esm bundles when packaging #2524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23fa147
feat(react): provide only umd & esm bundles when packaging
rarmatei Feb 21, 2020
1156aed
fix(react): print output error only once when bundling
rarmatei Feb 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions e2e/react-package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,16 @@ forEachCli('nx', cli => {
const childLib2Output = runCLI(`build ${childLib2}`);
const parentLibOutput = runCLI(`build ${parentLib}`);

expect(childLibOutput).toContain(`${childLib}.esm5.js`);
expect(childLibOutput).toContain(`${childLib}.esm.js`);
expect(childLibOutput).toContain(`${childLib}.umd.js`);
expect(childLibOutput).toContain(`Bundle complete`);

expect(childLib2Output).toContain(`${childLib2}.esm5.js`);
expect(childLib2Output).toContain(`${childLib2}.esm.js`);
expect(childLib2Output).toContain(`${childLib2}.umd.js`);
expect(childLib2Output).toContain(`Bundle complete`);

expect(parentLibOutput).toContain(`${parentLib}.esm5.js`);
expect(parentLibOutput).toContain(`${parentLib}.esm.js`);
expect(parentLibOutput).toContain(`${parentLib}.umd.js`);
expect(parentLibOutput).toContain(`Bundle complete`);

const jsonFile = readJson(`dist/libs/${parentLib}/package.json`);
Expand Down
23 changes: 21 additions & 2 deletions e2e/react.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,32 @@ forEachCli(currentCLIName => {
expect(libTestResults.stdout).toContain('Bundle complete.');

checkFilesExist(
`dist/libs/${libName}/package.json`,
`dist/libs/${libName}/index.d.ts`,
`dist/libs/${libName}/${libName}.esm5.js`,
`dist/libs/${libName}/${libName}.esm2015.js`,
`dist/libs/${libName}/${libName}.esm.js`,
`dist/libs/${libName}/${libName}.umd.js`
);
}, 120000);

it('should not create a dist folder if there is an error', async () => {
ensureProject();
const libName = uniq('lib');

runCLI(
`generate @nrwl/react:lib ${libName} --publishable --no-interactive`
);

const mainPath = `libs/${libName}/src/lib/${libName}.tsx`;
updateFile(mainPath, readFile(mainPath) + `\n console.log(a);`); // should error - "a" will be undefined

await expect(runCLIAsync(`build ${libName}`)).rejects.toThrow(
/Bundle failed/
);
expect(() => {
checkFilesExist(`dist/libs/${libName}/package.json`);
}).toThrow();
}, 120000);

it('should generate app with routing', async () => {
ensureProject();
const appName = uniq('app');
Expand Down
17 changes: 7 additions & 10 deletions packages/web/src/builders/package/package.impl.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ describe('WebPackagebuilder', () => {
success: true
});
});
spyOn(context.logger, 'info');

const result = await impl.run(testOptions, context).toPromise();

Expand All @@ -79,17 +80,13 @@ describe('WebPackagebuilder', () => {
},
{
format: 'esm',
file: '/root/dist/ui/example.esm2015.js',
name: 'Example'
},
{
format: 'esm',
file: '/root/dist/ui/example.esm5.js',
file: '/root/dist/ui/example.esm.js',
name: 'Example'
}
])
);
expect(result.success).toBe(true);
expect(context.logger.info).toHaveBeenCalledWith('Bundle complete.');
});

it('should return failure when one run fails', async () => {
Expand All @@ -99,10 +96,12 @@ describe('WebPackagebuilder', () => {
success: count++ === 0
});
});

spyOn(context.logger, 'error');
const result = await impl.run(testOptions, context).toPromise();

expect(result.success).toBe(false);
expect(f.writeJsonFile).not.toHaveBeenCalled();
expect(context.logger.error).toHaveBeenCalledWith('Bundle failed.');
});

it('updates package.json', async () => {
Expand All @@ -111,7 +110,6 @@ describe('WebPackagebuilder', () => {
success: true
});
});

await impl.run(testOptions, context).toPromise();

expect(f.writeJsonFile).toHaveBeenCalled();
Expand All @@ -120,8 +118,7 @@ describe('WebPackagebuilder', () => {
expect(content).toMatchObject({
name: 'example',
main: './example.umd.js',
module: './example.esm5.js',
es2015: './example.esm2015.js',
module: './example.esm.js',
typings: './index.d.ts'
});
});
Expand Down
80 changes: 34 additions & 46 deletions packages/web/src/builders/package/package.impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '@angular-devkit/architect';
import { JsonObject } from '@angular-devkit/core';
import { from, Observable, of } from 'rxjs';
import { map, mergeScan, switchMap, tap } from 'rxjs/operators';
import { switchMap, tap, last, mergeMap, catchError } from 'rxjs/operators';
import { runRollup } from './run-rollup';
import { createBabelConfig as _createBabelConfig } from '../../utils/babel-config';
import * as autoprefixer from 'autoprefixer';
Expand Down Expand Up @@ -42,15 +42,13 @@ export default createBuilder<BundleBuilderOptions & JsonObject>(run);

interface OutputConfig {
format: rollup.ModuleFormat;
esm: boolean;
extension: string;
declaration?: boolean;
}

const outputConfigs: OutputConfig[] = [
{ format: 'umd', esm: false, extension: 'umd' },
{ format: 'esm', esm: true, extension: 'esm2015' },
{ format: 'esm', esm: false, extension: 'esm5' }
{ format: 'umd', extension: 'umd' },
{ format: 'esm', extension: 'esm' }
];

const fileExtensions = ['.js', '.jsx', '.ts', '.tsx'];
Expand All @@ -72,7 +70,7 @@ export function run(
}
const options = normalizeBundleOptions(_options, context.workspaceRoot);
const rollupOptions: rollup.InputOptions[] = outputConfigs.map(
({ format, esm, extension }) => {
({ format, extension }, index) => {
const parsedTSConfig = ts.readConfigFile(
options.tsConfig,
ts.sys.readFile
Expand All @@ -83,6 +81,18 @@ export function run(
updatePaths(dependencies, parsedTSConfig.compilerOptions.paths);

const plugins = [
typescript({
check: true,
tsconfig: options.tsConfig,
tsconfigOverride: {
compilerOptions: {
rootDir: options.entryRoot,
allowJs: false,
declaration: index === 0,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only want to emit the declaration files the first time

paths: parsedTSConfig.compilerOptions.paths
}
}
}),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want to invoke the typescript compiler each time, so it can fail the build if something is wrong

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. I just wanna check to make sure the type error is only printed once rather than multiple times.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the build and it does output the same errors twice (e.g. type errors) if both rollup runs fail the same way. See my other comment for more on this.

#2524 (comment)

peerDepsExternal({
packageJsonPath: options.project
}),
Expand All @@ -95,31 +105,15 @@ export function run(
localResolve(),
resolve({ extensions: fileExtensions }),
babel({
...createBabelConfig(options, options.projectRoot, esm),
...createBabelConfig(options, options.projectRoot),
extensions: fileExtensions,
externalHelpers: false,
exclude: 'node_modules/**'
}),
commonjs(),
filesize()
];
if (esm) {
// TS plugin has to come first before types are stripped, otherwise
plugins.unshift(
typescript({
check: true,
tsconfig: options.tsConfig,
tsconfigOverride: {
compilerOptions: {
rootDir: options.entryRoot,
allowJs: false,
declaration: true,
paths: parsedTSConfig.compilerOptions.paths
}
}
})
);
}

const entryFileTmpl = `${options.outputPath}/${context.target.project}.<%= extension %>.js`;
const rollupConfig = {
input: options.entryFile,
Expand Down Expand Up @@ -166,21 +160,20 @@ export function run(
} else {
context.logger.info('Bundling...');
return from(rollupOptions).pipe(
mergeScan(
(acc, options) =>
runRollup(options).pipe(
map(result => {
return {
success: acc.success && result.success
};
})
),
{ success: true }
),
mergeMap(options => runRollup(options)),
catchError(e => {
console.error(e);
return of({ success: false });
}),
last(),
tap({
complete: () => {
updatePackageJson(options, context, target, dependencies);
context.logger.info('Bundle complete.');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously, if compilation (or anything else) failed, we'd still generate a dist folder with a package.json

next: result => {
if (result.success) {
updatePackageJson(options, context, target, dependencies);
context.logger.info('Bundle complete.');
} else {
context.logger.error('Bundle failed.');
}
}
})
);
Expand All @@ -191,12 +184,8 @@ export function run(

// -----------------------------------------------------------------------------

function createBabelConfig(
options: BundleBuilderOptions,
projectRoot: string,
esm: boolean
) {
let babelConfig: any = _createBabelConfig(projectRoot, esm, false);
function createBabelConfig(options: BundleBuilderOptions, projectRoot: string) {
let babelConfig: any = _createBabelConfig(projectRoot, false, false);
if (options.babelConfig) {
babelConfig = require(options.babelConfig)(babelConfig, options);
}
Expand Down Expand Up @@ -242,8 +231,7 @@ function updatePackageJson(options, context, target, dependencies) {
);
const packageJson = readJsonFile(options.project);
packageJson.main = entryFileTmpl.replace('<%= extension %>', 'umd');
packageJson.module = entryFileTmpl.replace('<%= extension %>', 'esm5');
packageJson.es2015 = entryFileTmpl.replace('<%= extension %>', 'esm2015');
packageJson.module = entryFileTmpl.replace('<%= extension %>', 'esm');
packageJson.typings = `./${typingsFile}`;
writeJsonFile(`${options.outputPath}/package.json`, packageJson);

Expand Down
6 changes: 1 addition & 5 deletions packages/web/src/builders/package/run-rollup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import { catchError, map, switchMap } from 'rxjs/operators';
export function runRollup(options: rollup.RollupOptions) {
return from(rollup.rollup(options)).pipe(
switchMap(bundle => from(bundle.write(options.output))),
map(() => ({ success: true })),
catchError(e => {
console.error(e);
return of({ success: false });
})
map(() => ({ success: true }))
);
}