Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): always enable looseEnums build …
Browse files Browse the repository at this point in the history
…optimizer rule

With this change TypeScript enums are always processing in "loose" mode. Previously, this was only done for `@angular/` packages.

(cherry picked from commit bf4229d)
  • Loading branch information
alan-agius4 committed Apr 27, 2023
1 parent dbb78f4 commit e690b7c
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import { NodePath, PluginObj, PluginPass, types } from '@babel/core';
import { NodePath, PluginObj, types } from '@babel/core';
import annotateAsPure from '@babel/helper-annotate-as-pure';

/**
Expand All @@ -27,10 +27,8 @@ export function getKeywords(): Iterable<string> {
export default function (): PluginObj {
return {
visitor: {
VariableDeclaration(path: NodePath<types.VariableDeclaration>, state: PluginPass) {
VariableDeclaration(path: NodePath<types.VariableDeclaration>) {
const { parentPath, node } = path;
const { loose } = state.opts as { loose: boolean };

if (node.kind !== 'var' || node.declarations.length !== 1) {
return;
}
Expand Down Expand Up @@ -80,13 +78,8 @@ export default function (): PluginObj {
const isEnumCalleeMatching =
types.isIdentifier(enumCalleeParam) && enumCalleeParam.name === declarationId.name;

// Loose mode rewrites the enum to a shorter but less TypeScript-like form
// Note: We only can apply the `loose` mode transformation if the callee parameter matches
// with the declaration identifier name. This is necessary in case the the declaration id has
// been renamed to avoid collisions, as the loose transform would then break the enum assignments
// which rely on the differently-named callee identifier name.
let enumAssignments: types.ExpressionStatement[] | undefined;
if (loose && isEnumCalleeMatching) {
if (isEnumCalleeMatching) {
enumAssignments = [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,11 @@ import { default as adjustTypeScriptEnums } from './adjust-typescript-enums';
// eslint-disable-next-line import/no-extraneous-dependencies
const prettier = require('prettier');

function testCase({
input,
expected,
options,
}: {
input: string;
expected: string;
options?: { loose?: boolean };
}): void {
function testCase({ input, expected }: { input: string; expected: string }): void {
const result = transform(input, {
configFile: false,
babelrc: false,
plugins: [[adjustTypeScriptEnums, options]],
plugins: [[adjustTypeScriptEnums]],
});
if (!result) {
fail('Expected babel to return a transform result.');
Expand Down Expand Up @@ -211,7 +203,7 @@ describe('adjust-typescript-enums Babel plugin', () => {
`);
});

it('wraps TypeScript enums in loose mode', () => {
it('wraps TypeScript enums', () => {
testCase({
input: `
var ChangeDetectionStrategy;
Expand All @@ -228,23 +220,19 @@ describe('adjust-typescript-enums Babel plugin', () => {
return ChangeDetectionStrategy;
})();
`,
options: { loose: true },
});
});

it(
'should not wrap TypeScript enums in loose mode if the declaration identifier has been ' +
'renamed to avoid collisions',
() => {
testCase({
input: `
it('should not wrap TypeScript enums if the declaration identifier has been renamed to avoid collisions', () => {
testCase({
input: `
var ChangeDetectionStrategy$1;
(function (ChangeDetectionStrategy) {
ChangeDetectionStrategy[ChangeDetectionStrategy["OnPush"] = 0] = "OnPush";
ChangeDetectionStrategy[ChangeDetectionStrategy["Default"] = 1] = "Default";
})(ChangeDetectionStrategy$1 || (ChangeDetectionStrategy$1 = {}));
`,
expected: `
expected: `
var ChangeDetectionStrategy$1 = /*#__PURE__*/ (() => {
(function (ChangeDetectionStrategy) {
ChangeDetectionStrategy[(ChangeDetectionStrategy["OnPush"] = 0)] = "OnPush";
Expand All @@ -253,8 +241,6 @@ describe('adjust-typescript-enums Babel plugin', () => {
return ChangeDetectionStrategy$1;
})();
`,
options: { loose: true },
});
},
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ export interface ApplicationPresetOptions {
inputSourceMap: unknown;
};
optimize?: {
looseEnums: boolean;
pureTopLevel: boolean;
wrapDecorators: boolean;
};
Expand Down Expand Up @@ -245,10 +244,7 @@ export default function (api: unknown, options: ApplicationPresetOptions) {

plugins.push(
require('../plugins/elide-angular-metadata').default,
[
require('../plugins/adjust-typescript-enums').default,
{ loose: options.optimize.looseEnums },
],
[require('../plugins/adjust-typescript-enums').default, { loose: true }],
[
require('../plugins/adjust-static-class-members').default,
{ wrapDecorators: options.optimize.wrapDecorators },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,15 @@ export default custom<ApplicationPresetOptions>(() => {
}

if (optimize) {
// @angular/platform-server/init entry-point has side-effects.
const safeAngularPackage =
/[\\/]node_modules[\\/]@angular[\\/]/.test(this.resourcePath) &&
!/@angular[\\/]platform-server[\\/]f?esm2022[\\/]init/.test(this.resourcePath);
const AngularPackage = /[\\/]node_modules[\\/]@angular[\\/]/.test(this.resourcePath);
const sideEffectFree = !!this._module?.factoryMeta?.sideEffectFree;
customOptions.optimize = {
// Angular packages provide additional tested side effects guarantees and can use
// otherwise unsafe optimizations.
looseEnums: safeAngularPackage,
pureTopLevel: safeAngularPackage,
// otherwise unsafe optimizations. (@angular/platform-server/init) however has side-effects.
pureTopLevel: AngularPackage && sideEffectFree,
// JavaScript modules that are marked as side effect free are considered to have
// no decorators that contain non-local effects.
wrapDecorators: !!this._module?.factoryMeta?.sideEffectFree,
wrapDecorators: sideEffectFree,
};

shouldProcess = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ async function transformWithBabel({
},
forceAsyncTransformation,
optimize: options.advancedOptimizations && {
looseEnums: angularPackage,
pureTopLevel: angularPackage,
},
},
Expand Down

0 comments on commit e690b7c

Please sign in to comment.