angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.72k stars 11.98k forks source link

AOT code folding bug in esbuild #26438

Closed Shosetsu closed 9 months ago

Shosetsu commented 10 months ago

Command

build

Is this a regression?

The previous version in which this bug was not present was

16.2.5 (used @angular-devkit/build-angular:browser)

Description

The same foldable source code unable to be folded when using esbuild. But its worked by build-angular:browser. If this bug is a feature when using esbuild, please update the description of aot in the guidelines document.

document refer: https://angular.dev/tools/cli/aot-compiler#foldable-syntax

Literal array ['cherries', 'flour', 'sugar'] yes

Array index ingredients[0] yes, if target and index are foldable

Minimal Reproduction

Minimal foldable source code:

// in external lib ts file
export const foldable = [{name: "obj1"}, {name: "obj2"}];
export const using = foldable[0];
// in angular application
import { using } from ...
export class AppComponent {
  title = using.name;
}

After ng build ...

17.0.0 (used @angular-devkit/build-angular:application)

  ry = [{ name: "obj1" }, { name: "obj2" }],
  od = ry[0];
  let t = class t {
    constructor() {
      this.title = od.name;
    }
  };

17.0.0 (used @angular-devkit/build-angular:browser)

  bj_name = "obj1";
  class e {
    constructor() {
      this.title = bj_name;
    }
  ...
  }

Exception or Error

No response

Your Environment

Angular CLI: 17.0.0
Node: 18.16.1
Package Manager: npm 9.7.1
OS: win32 x64

Angular: 17.0.2
... animations, common, compiler, compiler-cli, core, forms
... localize, platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.0
@angular-devkit/build-angular   17.0.0
@angular-devkit/core            17.0.0
@angular-devkit/schematics      17.0.0
@angular/cli                    17.0.0
@schematics/angular             17.0.0
ng-packagr                      17.0.0
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else relevant?

No response

clydin commented 10 months ago

The documentation referenced above pertains to the Angular compiler's partial evaluation capabilities to extract template and stylesheet constants at build time for AOT compilation. It does not have any effect on JavaScript outside the bounds of this specific Angular metadata analysis and does not perform any general purpose JavaScript optimization. The example code shown above is not related to a component's stylesheet nor template and as a result the documentation does not apply.

Shosetsu commented 10 months ago

Thanks clydin. I changed the minimal foldable source code to

import { using } from './test';
@Component({
  selector: 'app-root',
  standalone: true,
  imports: [CommonModule, RouterOutlet],
  template: `${using.name}`,
})
export class AppComponent {}

The code folding worked in esbuild mode. However, unused import constants are not removed. This is different from legacy browser mode.

var iy = [{ name: "obj1" }, { name: "obj2" }],
  Y0 = iy[0];
var ad = (() => {
  let t = class t {};
  (t.ɵfac = function (o) {
    return new (o || t)();
  }),
    (t.ɵcmp = Er({
      type: t,
      selectors: [["app-root"]],
      standalone: !0,
      features: [Pr],
      decls: 1,
      vars: 0,
      template: function (o, i) {
        o & 1 && Xc(0, "obj1");
      },
      dependencies: [Ki],
      encapsulation: 2,
    }));
  let e = t;
  return e;
})();

But I don't understand why unused import constants can't be removed in esbuild mode. I think it will only increase the size of the package.

clydin commented 9 months ago

The import will not be removed unless it is considered as side effect free. If it is in a library, that library's package.json should contain "sideEffects": false. Assuming the library is in fact side effect free. If it is part of the application code and the application code does not contain module-level non-local side effects, the same flag can be added to the project's package.json.

Shosetsu commented 9 months ago

Hi clydin, I tried to add "sideEffects": false to package.json, but nothing changed. I suppose the main reason for this difference is not unused import object removal. But rather, it's that in @angular-devkit/build-angular:browser the compiler folds the code in component.ts, but not in @angular-devkit/build-angular:application. As you said, even in esbuild mode AOT code folding can be executed correctly in the templates. However it seems that in browser mode the code in component.ts is also folded.

alan-agius4 commented 9 months ago

Hi @Shosetsu, in the above case, with the webpack builder unused properties from the object example { name: "obj2" } will only be dropped if the const/variable is referred only 1 time in the entire codebase. This type of optimization is handled by terser which by design is not part with the esbuild pipeline.

Esbuild does not perform this sort of optimization on objects and arrays. That said, I'd also say it's quite an edge case, that one would have an object/array with multiple properties/items that are unused and it's only referenced once in the entire code base. At this point, why not clean up the object manually?

Input

const obj = [{ name: "obj1" }, { name: "obj2" }];
console.log(obj[0])

output

console.log({name:"obj1"})

Input

const obj = [{ name: "obj1" }, { name: "obj2" }];
console.log(obj[0])
console.log(obj[0])

output

const o=[{name:"obj1"},{name:"obj2"}];
console.log(o[0]),
console.log(o[0])
angular-automatic-lock-bot[bot] commented 8 months ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.