[Case Study] Angular CLI

bgotink commented 5 years ago

What package is covered by this investigations?

All packages contained in the angular/angular-cli repository, most notably @angular/cli.

Describe the goal of the investigation

As an angular developer I want my builds to work with berry (or PnP in general). Not having this would mean I can't use yarn and I don't want to even think about the consequences to that as I use yarn workspaces in literally all of my angular repos.

Angular has a work-in-progress bazel buildchain. Bazel uses yarn internally, but getting that up and running with berry is best left to another ticket.

Investigation report

Test repo at bgotink/berry-angular-poc. Current status: not working at all

Encountered issues:

A custom module resolution in @angular-devkit/core/node/resolve. I've hacked together a quick resolution function using pnpapi here, that seems to work for now though I'm sure I'll run into more issues later.
~~Next up is webpack-subresource-integrity which is at a version that doesn't list html-webpack-plugin as peer dependency, leading to an error.~~ ~~Adding a resolution to the first version that does include the missing peer dependency leads to #265.~~ Fixed by #279

bgotink commented 5 years ago

I've updated yarn to master to include the #279, the build is still broken of course :)

Updates:

Webpack configuration needs to be modified to load pnp-webpack-plugin (until enhanced-resolve is updated to v5 and webpack updated to match)

Issue when trying to build: pnp errors while trying to load builtin modules via webpack:

ERROR in (webpack)/hot/emitter.js
Module not found: Error: A package is trying to access another package without the second one being listed as a dependency of the first one

Required package: events (via "events")
Required by: webpack@npm:4.30.0 (via /Volumes/Workspaces/Private/berry-angular/.yarn/cache/webpack-npm-4.30.0-755355621c00c2d22785f1e627ca51939cad51b16c530b4cd2328dd8e2319f18.zip/node_modules/webpack/hot/emitter.js)

ERROR in ./.yarn/virtual/webpack-dev-server-virtual-aaeff65718653d2d0ca543f1c4e28c8cf07bccfd1f166fc97aa4a5c6265fe982/node_modules/webpack-dev-server/client?http://0.0.0.0:0/sockjs-node
Module not found: Error: A package is trying to access another package without the second one being listed as a dependency of the first one

Required package: querystring (via "querystring")
Required by: webpack-dev-server@virtual:df6aabc6e52f59494f85e743d904dd7e1d08df72963cecf9ef41b4818a6d0d00d3dc042b36dc1654888f4b297369334d8cc43963d47c852d2355300877f4cc64#npm:3.3.1 (via /Volumes/Workspaces/Private/berry-angular/.yarn/virtual/webpack-dev-server-virtual-aaeff65718653d2d0ca543f1c4e28c8cf07bccfd1f166fc97aa4a5c6265fe982/node_modules/webpack-dev-server/client/index.js)

bgotink commented 5 years ago

Updates:

I'm still getting the errors for the two builtin modules above. I don't know why these don't work? Using yarn ng build instead of yarn ng serve seems to work around it by skipping webpack-dev-server.
Updated angular to the latest version, now I'm getting errors because it tries to load /path/to/berry-angular/node_modules/@angular/common/http.ngsummary.json. This leads to
```
ERROR in : Unexpected value 'HttpClientModule in /path/to/berry-angular/node_modules/@angular/common/http/http.d.ts' imported by the module 'AppModule in /path/to/berry-angular/src/app/app.module.ts'. Please add a @NgModule annotation.
```
Note that if I remove HttpClientModule I get the error for BrowserModule and /path/to/berry-angular/node_modules/@angular/platform-browser/platform-browser.d.ts instead even though in that case I don't see the program trying to load a .ngsummary.json file.
ng update leads to an out-of-memory error
Error stack
``` $ yarn ng update @angular/cli @angular/core Using package manager: 'yarn' Collecting installed dependencies... <--- Last few GCs ---> [92456:0x10264e000] 51889 ms: Mark-sweep 1356.4 (1441.2) -> 1345.3 (1442.2) MB, 612.3 / 0.6 ms (average mu = 0.199, current mu = 0.107) allocation failure scavenge might not succeed [92456:0x10264e000] 52573 ms: Mark-sweep 1358.6 (1442.2) -> 1343.8 (1441.7) MB, 643.4 / 0.6 ms (average mu = 0.133, current mu = 0.059) allocation failure scavenge might not succeed <--- JS stacktrace ---> ==== JS stack trace ========================================= 0: ExitFrame [pc: 0x68a992dbe3d] 1: StubFrame [pc: 0x68a99291166] Security context: 0x0993ddd1e6e9
2: relative [0x99333bad609] [path.js:~1161] [pc=0x68a9937cd45](this=0x099333b82201
,from=0x0993e0704ec9
,to=0x0993e013e9a1 ) 3: findPackageLocator(aka findPackageLocator) ... FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory 1: 0x10003c597 node::Abort() [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 2: 0x10003c7a1 node::OnFatalError(char const*, char const*) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 3: 0x1001ad575 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 4: 0x100579242 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 5: 0x10057bd15 v8::internal::Heap::CheckIneffectiveMarkCompact(unsigned long, double) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 6: 0x100577bbf v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 7: 0x100575d94 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 8: 0x10058262c v8::internal::Heap::AllocateRawWithLigthRetry(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 9: 0x1005826af v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 10: 0x100551ff4 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 11: 0x1007da044 v8::internal::Runtime_AllocateInNewSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/Users/bram/.nvm/versions/node/v10.15.3/bin/node] 12: 0x68a992dbe3d 13: 0x68a99291166 14: 0x68a9937cd45 15: 0x68a994aa9dc ```

Embraser01 commented 5 years ago

Issue when trying to build: pnp errors while trying to load builtin modules via webpack:

ERROR in (webpack)/hot/emitter.js Module not found: Error: A package is trying to access another package without the second one being listed as a dependency of the first one Required package: events (via "events") Required by: webpack@npm:4.30.0 (via /Volumes/Workspaces/Private/berry-angular/.yarn/cache/webpack-npm-4.30.0-755355621c00c2d22785f1e627ca51939cad51b16c530b4cd2328dd8e2319f18.zip/node_modules/webpack/hot/emitter.js) ERROR in ./.yarn/virtual/webpack-dev-server-virtual-aaeff65718653d2d0ca543f1c4e28c8cf07bccfd1f166fc97aa4a5c6265fe982/node_modules/webpack-dev-server/client?http://0.0.0.0:0/sockjs-node Module not found: Error: A package is trying to access another package without the second one being listed as a dependency of the first one Required package: querystring (via "querystring") Required by: webpack-dev-server@virtual:df6aabc6e52f59494f85e743d904dd7e1d08df72963cecf9ef41b4818a6d0d00d3dc042b36dc1654888f4b297369334d8cc43963d47c852d2355300877f4cc64#npm:3.3.1 (via /Volumes/Workspaces/Private/berry-angular/.yarn/virtual/webpack-dev-server-virtual-aaeff65718653d2d0ca543f1c4e28c8cf07bccfd1f166fc97aa4a5c6265fe982/node_modules/webpack-dev-server/client/index.js)

So for the events package, it should be resolved in webpack 5 as a PR has already be merged into the next branch of webpack.

For querystring we should probably make a PR on webpack/webpack-dev-server to add it to the dependencies 🙂

The build succeeded on the latest commit of your repo. I tried ng update and ng update @angular/cli and it didn't go out of memory but I got warnings about the npm-registry-fetch that requires safe-buffer without declaring it.

I didn't upgraded angular to the latest version yet... I'll try to dig more into it by the end of the month :wink:

bgotink commented 5 years ago

I forgot to push some changes apparently, consider that rectified.

Due to a busy work schedule I haven't been able to look into this some more, and it doesn't look like the next week(s) are going to be an improvement. If you want to dig deeper, by all means :)

Embraser01 commented 5 years ago

So it seems that upgrading angular to the latest version (8.2.7) have fixed the yarn ng serve problem.

I'm looking now for the out of memory problem and it seems that it comes from the read-package-tree module called by angular cli...

The package does not work with PnPify

arcanis commented 5 years ago

@Embraser01 do you mind submitting a PR for an E2E test case like the ones we have in the repo? Even if it doesn't pass yet, it would make it easier for people unfamiliar with Angular (such as me :p) to know how to quickly reproduce what is meant to work but doesn't 😃

Embraser01 commented 5 years ago

Yep, was thinking about making an E2E test! Will try to make one tomorrow 😉

larixer commented 5 years ago

@Embraser01 I have briefly looked at pnpify+read-package-tree. And the thing is read-package-tree tries to read all the node_modules tree recursively. Now in the case of pnpify, it creates the tree with pnp guarantees in memory, which is a much much larger tree then hoisted node_modules tree. So pnpify kinda does its job correctly. To support react-package-tree in pnpify, the pnpify should in addition to pnp-tree be able to build hoisted node_modules tree without pnp-guarantees, (via some cli flag, say --hoist). Of course it would be much better if read-package-tree had native pnp-support, but it should have other design for iterating over pnp-tree, because pnp-tree is a very fast growing tree....

Embraser01 commented 5 years ago

So the E2E test works for the build command... In order to make it work:

npm install -g @angular/cli Install the latest version (+8.3)

ng config -g cli.packageManager yarn

ng new berry-angular For now the yarn install step fails because of the --silent option

Create a patch file replacing the @angular/cli/bin/ng file.

yarn add @yarnpkg/pnpify @angular-devkit/core @babel/preset-env @babel/core -D

Now to run a ng command: yarn pnpify ./angular-patch.js build|serve|...|new

bgotink commented 4 years ago

I've opened a couple of tickets to kickstart things upstream in the angular-cli repo:

https://github.com/angular/angular-cli/issues/16979

https://github.com/angular/angular-cli/issues/16981

There's also this existing ticket which currently leads to a failure in ng version

https://github.com/angular/angular-cli/issues/10917

Currently I'm using a yarn plugin (https://github.com/bgotink/yarn-plugin-angular) to get everything up and running (without pnpify) and to identify what needs to change to get everything up and running. Hopefully that plugin is short-lived!

bgotink commented 4 years ago

And there's now a ticket on angular's side to track yarn 2 support: https://github.com/angular/angular-cli/issues/16980

© Githubissues.

Githubissues is a development platform for aggregating issues.

yarnpkg / berry

[Case Study] Angular CLI #266