Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): treeshake unused class that use c…
Browse files Browse the repository at this point in the history
…ustom decorators

This changes enables wrapping classes in side-effect free modules that make use of custom decorators when using the esbuild based builders so that when such classes are unused they can be treeshaken.

(cherry picked from commit 7b9b7fe)
  • Loading branch information
alan-agius4 committed Nov 22, 2023
1 parent 7e12fdf commit 657a07b
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,12 @@ export function createCompilerPlugin(
};
} else if (typeof contents === 'string') {
// A string indicates untransformed output from the TS/NG compiler
const sideEffects = await hasSideEffects(request);
contents = await javascriptTransformer.transformData(
request,
contents,
true /* skipLinker */,
sideEffects,
);

// Store as the returned Uint8Array to allow caching the fully transformed code
Expand All @@ -380,9 +382,11 @@ export function createCompilerPlugin(
return profileAsync(
'NG_EMIT_JS*',
async () => {
const sideEffects = await hasSideEffects(args.path);
const contents = await javascriptTransformer.transformFile(
args.path,
pluginOptions.jit,
sideEffects,
);

return {
Expand Down Expand Up @@ -430,6 +434,22 @@ export function createCompilerPlugin(
void stylesheetBundler.dispose();
void compilation.close?.();
});

/**
* Checks if the file has side-effects when `advancedOptimizations` is enabled.
*/
async function hasSideEffects(path: string): Promise<boolean | undefined> {
if (!pluginOptions.advancedOptimizations) {
return undefined;
}

const { sideEffects } = await build.resolve(path, {
kind: 'import-statement',
resolveDir: build.initialOptions.absWorkingDir ?? '',
});

return sideEffects;
}
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ interface JavaScriptTransformRequest {
sourcemap: boolean;
thirdPartySourcemaps: boolean;
advancedOptimizations: boolean;
skipLinker: boolean;
skipLinker?: boolean;
sideEffects?: boolean;
jit: boolean;
}

Expand Down Expand Up @@ -50,11 +51,8 @@ async function transformWithBabel({
return useInputSourcemap ? data : data.replace(/^\/\/# sourceMappingURL=[^\r\n]*/gm, '');
}

// `@angular/platform-server/init` and `@angular/common/locales/global` entry-points are side effectful.
const safeAngularPackage =
/[\\/]node_modules[\\/]@angular[\\/]/.test(filename) &&
!/@angular[\\/]platform-server[\\/]f?esm2022[\\/]init/.test(filename) &&
!/@angular[\\/]common[\\/]locales[\\/]global/.test(filename);
const sideEffectFree = options.sideEffects === false;
const safeAngularPackage = sideEffectFree && /[\\/]node_modules[\\/]@angular[\\/]/.test(filename);

// Lazy load the linker plugin only when linking is required
if (shouldLink) {
Expand Down Expand Up @@ -85,6 +83,7 @@ async function transformWithBabel({
},
optimize: options.advancedOptimizations && {
pureTopLevel: safeAngularPackage,
wrapDecorators: sideEffectFree,
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,14 @@ export class JavaScriptTransformer {
* If no transformations are required, the data for the original file will be returned.
* @param filename The full path to the file.
* @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking.
* @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped.
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
*/
transformFile(filename: string, skipLinker?: boolean): Promise<Uint8Array> {
transformFile(
filename: string,
skipLinker?: boolean,
sideEffects?: boolean,
): Promise<Uint8Array> {
const pendingKey = `${!!skipLinker}--${filename}`;
let pending = this.#pendingfileResults?.get(pendingKey);
if (pending === undefined) {
Expand All @@ -83,6 +88,7 @@ export class JavaScriptTransformer {
pending = this.#ensureWorkerPool().run({
filename,
skipLinker,
sideEffects,
...this.#commonOptions,
});

Expand All @@ -98,9 +104,15 @@ export class JavaScriptTransformer {
* @param filename The full path of the file represented by the data.
* @param data The data of the file that should be transformed.
* @param skipLinker If true, bypass all Angular linker processing; if false, attempt linking.
* @param sideEffects If false, and `advancedOptimizations` is enabled tslib decorators are wrapped.
* @returns A promise that resolves to a UTF-8 encoded Uint8Array containing the result.
*/
async transformData(filename: string, data: string, skipLinker: boolean): Promise<Uint8Array> {
async transformData(
filename: string,
data: string,
skipLinker: boolean,
sideEffects?: boolean,
): Promise<Uint8Array> {
// Perform a quick test to determine if the data needs any transformations.
// This allows directly returning the data without the worker communication overhead.
if (skipLinker && !this.#commonOptions.advancedOptimizations) {
Expand All @@ -118,6 +130,7 @@ export class JavaScriptTransformer {
filename,
data,
skipLinker,
sideEffects,
...this.#commonOptions,
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import assert from 'assert';
import { appendToFile, expectFileToExist, expectFileToMatch, readFile } from '../../../utils/fs';
import { ng } from '../../../utils/process';
import { libraryConsumptionSetup } from './setup';
import { updateJsonFile } from '../../../utils/project';
import { expectToFail } from '../../../utils/utils';

export default async function () {
await ng('cache', 'off');
await libraryConsumptionSetup();

// Add an unused class as part of the public api.
await appendToFile(
'projects/my-lib/src/public-api.ts',
`
function something() {
return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
console.log("someDecorator");
};
}
export class ExampleClass {
@something()
method() {}
}
`,
);

// build the lib
await ng('build', 'my-lib', '--configuration=production');
const packageJson = JSON.parse(await readFile('dist/my-lib/package.json'));
assert.equal(packageJson.sideEffects, false);

// build the app
await ng('build', 'test-project', '--configuration=production', '--output-hashing=none');
// Output should not contain `ExampleClass` as the library is marked as side-effect free.
await expectFileToExist('dist/test-project/browser/main.js');
await expectToFail(() => expectFileToMatch('dist/test-project/browser/main.js', 'someDecorator'));

// Mark library as side-effectful.
await updateJsonFile('dist/my-lib/package.json', (packageJson) => {
packageJson.sideEffects = true;
});

// build the app
await ng('build', 'test-project', '--configuration=production', '--output-hashing=none');
// Output should contain `ExampleClass` as the library is marked as side-effectful.
await expectFileToMatch('dist/test-project/browser/main.js', 'someDecorator');
}

0 comments on commit 657a07b

Please sign in to comment.