Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): correctly resolve polyfills when …
Browse files Browse the repository at this point in the history
…`baseUrl` URL is not set to root

Prior to this change when `baseUrl` was set to non-root or not set polyfills were not correctly resolved. Internally Esbuild uses the `baseUrl` to resolve non relative imports.

Closes: #25341
  • Loading branch information
alan-agius4 authored and clydin committed Oct 5, 2023
1 parent 863e142 commit 667f43a
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,61 +9,78 @@
import { buildApplication } from '../../index';
import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup';

const testsVariants: [suitName: string, baseUrl: string | undefined][] = [
['When "baseUrl" is set to "./"', './'],
[`When "baseUrl" is not set`, undefined],
[`When "baseUrl" is set to non root path`, './project/foo'],
];

describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
describe('Option: "polyfills"', () => {
it('uses a provided TypeScript file', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/polyfills.ts'],
for (const [suitName, baseUrl] of testsVariants) {
describe(suitName, () => {
beforeEach(async () => {
await harness.modifyFile('tsconfig.json', (content) => {
const tsconfig = JSON.parse(content);
tsconfig.compilerOptions.baseUrl = baseUrl;

return JSON.stringify(tsconfig);
});
});

const { result } = await harness.executeOnce();

expect(result?.success).toBe(true);
it('uses a provided TypeScript file', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/polyfills.ts'],
});

harness.expectFile('dist/polyfills.js').toExist();
});
const { result } = await harness.executeOnce();

it('uses a provided JavaScript file', async () => {
await harness.writeFile('src/polyfills.js', `console.log('main');`);
expect(result?.success).toBe(true);

harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/polyfills.js'],
harness.expectFile('dist/polyfills.js').toExist();
});

const { result } = await harness.executeOnce();
it('uses a provided JavaScript file', async () => {
await harness.writeFile('src/polyfills.js', `console.log('main');`);

expect(result?.success).toBe(true);
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/polyfills.js'],
});

harness.expectFile('dist/polyfills.js').content.toContain(`console.log("main")`);
});
const { result } = await harness.executeOnce();

it('fails and shows an error when file does not exist', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/missing.ts'],
expect(result?.success).toBe(true);

harness.expectFile('dist/polyfills.js').content.toContain(`console.log("main")`);
});

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
it('fails and shows an error when file does not exist', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['src/missing.ts'],
});

expect(result?.success).toBe(false);
expect(logs).toContain(
jasmine.objectContaining({ message: jasmine.stringMatching('Could not resolve') }),
);
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });

harness.expectFile('dist/polyfills.js').toNotExist();
});
expect(result?.success).toBe(false);
expect(logs).toContain(
jasmine.objectContaining({ message: jasmine.stringMatching('Could not resolve') }),
);

it('resolves module specifiers in array', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['zone.js', 'zone.js/testing'],
harness.expectFile('dist/polyfills.js').toNotExist();
});

const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
harness.expectFile('dist/polyfills.js').toExist();
it('resolves module specifiers in array', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['zone.js', 'zone.js/testing'],
});

const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
harness.expectFile('dist/polyfills.js').toExist();
});
});
});
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { BuildOptions } from 'esbuild';
import assert from 'node:assert';
import { createHash } from 'node:crypto';
import { readFile } from 'node:fs/promises';
import { join, relative } from 'node:path';
import { extname, join, relative } from 'node:path';
import type { NormalizedApplicationBuildOptions } from '../../builders/application/options';
import { allowMangle } from '../../utils/environment-options';
import { SourceFileCache, createCompilerPlugin } from './angular/compiler-plugin';
Expand Down Expand Up @@ -112,11 +112,31 @@ export function createBrowserCodeBundleOptions(
buildOptions.plugins?.unshift(
createVirtualModulePlugin({
namespace,
loadContent: () => ({
contents: polyfills.map((file) => `import '${file.replace(/\\/g, '/')}';`).join('\n'),
loader: 'js',
resolveDir: workspaceRoot,
}),
loadContent: async (_, build) => {
const polyfillPaths = await Promise.all(
polyfills.map(async (path) => {
if (path.startsWith('zone.js') || !extname(path)) {
return path;
}

const potentialPathRelative = './' + path;
const result = await build.resolve(potentialPathRelative, {
kind: 'import-statement',
resolveDir: workspaceRoot,
});

return result.path ? potentialPathRelative : path;
}),
);

return {
contents: polyfillPaths
.map((file) => `import '${file.replace(/\\/g, '/')}';`)
.join('\n'),
loader: 'js',
resolveDir: workspaceRoot,
};
},
}),
);
}
Expand Down

0 comments on commit 667f43a

Please sign in to comment.