Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): use a dash in bundle names
Browse files Browse the repository at this point in the history
This updates the esbuild based builder to use a dash in bundles and media instead of a dot to be consistent with the chunks files `chunk-xxx.js`.
  • Loading branch information
cexbrayat authored and alan-agius4 committed Sep 21, 2023
1 parent f9fdd09 commit 4e89c3c
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ export async function normalizeOptions(
const outputNames = {
bundles:
options.outputHashing === OutputHashing.All || options.outputHashing === OutputHashing.Bundles
? '[name].[hash]'
? '[name]-[hash]'
: '[name]',
media:
'media/' +
(options.outputHashing === OutputHashing.All || options.outputHashing === OutputHashing.Media
? '[name].[hash]'
? '[name]-[hash]'
: '[name]'),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /main\.[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /polyfills\.[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeTrue();
expect(harness.hasFileMatch('dist/media', /spectrum\.[0-9A-Z]{8}\.png$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /main-[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /polyfills-[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeTrue();
expect(harness.hasFileMatch('dist/media', /spectrum-[0-9A-Z]{8}\.png$/)).toBeTrue();
});

it(`doesn't hash any filenames when not set`, async () => {
Expand All @@ -49,10 +49,10 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /main\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum\.[0-9A-Z]{8}\.png$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /main-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum-[0-9A-Z]{8}\.png$/)).toBeFalse();
});

it(`doesn't hash any filenames when set to "none"`, async () => {
Expand All @@ -68,10 +68,10 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /main\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum\.[0-9A-Z]{8}\.png$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /main-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum-[0-9A-Z]{8}\.png$/)).toBeFalse();
});

it(`hashes CSS resources filenames only when set to "media"`, async () => {
Expand All @@ -87,10 +87,10 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /main\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills\.[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum\.[0-9A-Z]{8}\.png$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /main-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /polyfills-[0-9A-Z]{8}\.js$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist/media', /spectrum-[0-9A-Z]{8}\.png$/)).toBeTrue();
});

it(`hashes bundles filenames only when set to "bundles"`, async () => {
Expand All @@ -106,10 +106,10 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /main\.[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /polyfills\.[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeTrue();
expect(harness.hasFileMatch('dist/media', /spectrum\.[0-9A-Z]{8}\.png$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /main-[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /polyfills-[0-9A-Z]{8}\.js$/)).toBeTrue();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeTrue();
expect(harness.hasFileMatch('dist/media', /spectrum-[0-9A-Z]{8}\.png$/)).toBeFalse();
});

it('does not hash non injected styles', async () => {
Expand All @@ -128,8 +128,8 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
const { result } = await harness.executeOnce();
expect(result?.success).toBe(true);

expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles\.[0-9A-Z]{8}\.css.map$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css$/)).toBeFalse();
expect(harness.hasFileMatch('dist', /styles-[0-9A-Z]{8}\.css.map$/)).toBeFalse();
harness.expectFile('dist/styles.css').toExist();
harness.expectFile('dist/styles.css.map').toExist();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ export class BundlerContext {
outputFile.path = relativeFilePath;

if (entryPoint) {
// The first part of the filename is the name of file (e.g., "polyfills" for "polyfills.7S5G3MDY.js")
const name = basename(relativeFilePath).split('.', 1)[0];
// The first part of the filename is the name of file (e.g., "polyfills" for "polyfills-7S5G3MDY.js")
const name = basename(relativeFilePath).replace(/(?:-[\dA-Z]{8})?\.[a-z]{2,3}$/, '');
// Entry points are only styles or scripts
const type = extname(relativeFilePath) === '.css' ? 'style' : 'script';

Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/basic/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export default async function () {
// Production build
const { stdout: stdout2 } = await ng('build');
if (getGlobalVariable('argv')['esbuild']) {
// esbuild uses an 8 character hash
await expectFileToMatch('dist/test-project/index.html', /main\.[a-zA-Z0-9]{8}\.js/);
// esbuild uses an 8 character hash and a dash as separator
await expectFileToMatch('dist/test-project/index.html', /main-[a-zA-Z0-9]{8}\.js/);
} else {
await expectFileToMatch('dist/test-project/index.html', /main\.[a-zA-Z0-9]{16}\.js/);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/basic/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export default async function () {
// Production build
await silentNg('run', 'test-project:build');
if (getGlobalVariable('argv')['esbuild']) {
// esbuild uses an 8 character hash
await expectFileToMatch('dist/test-project/index.html', /main\.[a-zA-Z0-9]{8}\.js/);
// esbuild uses an 8 character hash and a dash as separator
await expectFileToMatch('dist/test-project/index.html', /main-[a-zA-Z0-9]{8}\.js/);
} else {
await expectFileToMatch('dist/test-project/index.html', /main\.[a-zA-Z0-9]{16}\.js/);
}
Expand Down
16 changes: 13 additions & 3 deletions tests/legacy-cli/e2e/tests/build/prod-build.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { statSync } from 'fs';
import { join } from 'path';
import { getGlobalVariable } from '../../utils/env';
import { expectFileToExist, expectFileToMatch, readFile } from '../../utils/fs';
import { noSilentNg } from '../../utils/process';

Expand Down Expand Up @@ -30,12 +31,21 @@ export default async function () {
await noSilentNg('build');
await expectFileToExist(join(process.cwd(), 'dist'));
// Check for cache busting hash script src
await expectFileToMatch('dist/test-project/index.html', /main\.[0-9a-zA-Z]{8,16}\.js/);
await expectFileToMatch('dist/test-project/index.html', /styles\.[0-9a-zA-Z]{8,16}\.css/);
if (getGlobalVariable('argv')['esbuild']) {
// esbuild uses an 8 character hash and a dash as separator
await expectFileToMatch('dist/test-project/index.html', /main-[0-9a-zA-Z]{8}\.js/);
await expectFileToMatch('dist/test-project/index.html', /styles-[0-9a-zA-Z]{8}\.css/);
} else {
await expectFileToMatch('dist/test-project/index.html', /main\.[0-9a-zA-Z]{16}\.js/);
await expectFileToMatch('dist/test-project/index.html', /styles\.[0-9a-zA-Z]{16}\.css/);
}
await expectFileToMatch('dist/test-project/3rdpartylicenses.txt', /MIT/);

const indexContent = await readFile('dist/test-project/index.html');
const mainPath = indexContent.match(/src="(main\.[0-9a-zA-Z]{0,32}\.js)"/)![1];
const mainSrcRegExp = getGlobalVariable('argv')['esbuild']
? /src="(main-[0-9a-zA-Z]{8}\.js)"/
: /src="(main\.[0-9a-zA-Z]{16}\.js)"/;
const mainPath = indexContent.match(mainSrcRegExp)![1];

// Content checks
await expectFileToMatch(`dist/test-project/${mainPath}`, bootstrapRegExp);
Expand Down

0 comments on commit 4e89c3c

Please sign in to comment.