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.
  • Loading branch information
alan-agius4 committed Nov 22, 2023
1 parent 6473b01 commit 7b9b7fe
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 7b9b7fe

Please sign in to comment.