build-optimizer hardcodes private "esm5" paths to enable optimizations

[IgorMinar]

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting

Area

- [x] devkit

Versions

Angular CLI: 1.6.6
Node: 8.9.4
OS: darwin x64
Angular: 6.0.0-beta.7-ac402a5a85
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

@angular/cli: 1.6.6 (but the problematic code is still present in master)
@angular-devkit/build-optimizer: 0.0.42
@angular-devkit/core: 0.0.29
@angular-devkit/schematics: 0.0.52
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.6
@schematics/angular: 0.1.17
typescript: 2.7.2

Repro steps

the following line hardcodes "esm5" path - which is an implementation detail and not part of the public api:

devkit/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts

Line 44 in 40c5eeb

/[\\/]node_modules[\\/]@angular[\\/][^\\/]+[\\/]esm5[\\/]/,

this line then uses the path above to test if build-optimizer optimizations should be enabled:

devkit/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts

Line 59 in 40c5eeb

&& es5AngularModules.some((re) => re.test(filePath))

Desired functionality

it's not clear to me why we hardcode these paths. ideally we should follow the "module" property to see where the code is in the package, or even better we should check the new "sideEffects" property introduced by webpack v4.

It seems that we are intentionally excluding the es2015 distribution from these optimizations. I'm not sure why, but regardless of the reason, relying on paths is not the right way to go about this - we should be consulting the package.json and using the same resolution algorithm (or the reversed version of it) to understand what kind of file we are dealing with.

Mention any other details that might be useful

[filipesilva]

@IgorMinar the reason those paths are hardcoded is because Build Optimizer itself does not know about entry points as described in package.json. It only knows about file paths.

When used with Webpack, that information could be passed along. But Build Optimizer can be used outside of Webpack as well. Doing resolution for each file passed can be costly so that determination is left to the tooling.

I think the right approach to move out of the hardcoded paths is to make use of the isSideEffectFree flag on the consuming tooling: https://github.com/angular/devkit/blob/master/packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts#L70

This is what we used for the Bazel integration via Rollup. The webpack loader hasn't yet been revisited to use this as well.

The es2015 are excluded intentionally because Build Optimizer doesn't support es2015 code, with some of the optimizations breaking it.

[IgorMinar]

aha! great insights. thanks! I agree that we should just switch over to detection based on the sideEffects package.json flag.

[clydin]

PR #531 allows the build optimizer to leverage the webpack side effect detection.

[clydin]

Also of note: The CLI v6 webpack 4 configuration marks previous versions of Angular as side effect free in addition to any packages with the sideEffects flag.