feat(@ngtools/webpack): add NGCC as part of the workflow - #13968

Blocked until the framework release the NGCC API in the next beta version.

@alan-agius4 could you include the reasoning for not having ngtsc be in change of running ngcc, that you mention from the sync meeting?

Discussed in standup this morning and @kara and @mhevery think it's an interesting idea to run it this way

@alexeagle the main reasoning that I got for not doing this was not to make ngtsc dependent on ngcc.

Though maybe there is more to it @petebacondarwin & @alxhub would know more.

Also this won’t be transparent for us, as ngtsc will need to return the packages that have been transformed by ngcc so that we can purge them from cache.

This PR is effectively getting ngtsc to trigger ngcc.

Already the CLI is providing a specially configured TypeScript CompilerHost to ngtsc, which it then uses to do its processing.

Here we are adding additional customisation to this CompilerHost that also triggers ngcc when module resolution is taking place. This is the right place to do this processing, and is also the best API for customising the ngtsc compilation behaviour.

The alternative of adding some kind of flag to the CompilerOptions, which then requires ngtsc to add this ngcc processing into the CompilerHost that it is handed is not ideal:

1. It makes ngtsc aware of ngcc, which is not desirable and makes ngtsc harder to maintain (and test).
2. It would require ngtsc to have significant additional code that would have to override the CompilerHost that is passed to it, which could create conflicts of interest.
3. It adds bloat to ngtsc for a use case which is specific to CLI (and would need to be removed if/when ngcc is no longer needed.

Not to disagree with the other points, but running ngcc transparently and only when needed isn't a use case that is specific to the CLI. It would in fact be the better way of running ngcc overall.

Thinking about it some more I also do not understand the maintenance argument. ngcc is meant to be used with ngtsc as part of @angular/compiler-cli exposed in the public API, thus subject to current deprecation policies. It will be around for a while.

As it stands the maintenance cost for ngcc is distributed between @angular/compiler-cli and projects/tooling that consume it, either via the post install hook or this Angular CLI hook. You might regard the postinstall hook as not requiring maintenance, but it does: it must be added and removed manually, and will not work outside certain version ranges of @angular/compiler-cli.

But any usage is completely bound to the API exposed by @angular/compiler-cli. So if the API in @angular/compiler-cli changes then consumers must support both new and old API, as projects can be using either.

This is to say that confining maintenance cost to @angular/compiler-cli is preferable as it reduces total maintenance and requires no updates in other projects/tooling efforts.

Thanks for your input @filipesilva. What you say makes a lot of sense.

There is definitely value in avoiding significant external configuration of ngtsc for build tools like webpack. We designed ngcc as a standalone tool in the first place to avoid having to publish multiple integrations with build tools (of which webpack is one).

I think the concern is that each integration might need to configure things (like the CompilerHost) slightly differently, and so it would be better to let them do so directly rather than trying to apply a one-size-fits-all feature in ngtsc.

My understanding is that the life of ngcc is only until we have moved to ivy as the default, at which point we would expect libraries to start publishing their distributables compiled for ivy directly.

Despite all of this, I wonder if it might be worth publishing a wrapper around ngc/ngtsc, which basically does the customisation of the CompilerHost similar to what we are doing here. Tools could then choose to use this wrapper rather than the "standard" ngc that we have right now.

I think combining the two tools is the wrong approach, because their missions and requirements are so different.

ngtsc is a thin wrapper around the TypeScript compiler. It has one goal: to turn your .ts files into .js files. It tries to do this as efficiently as possible, independently of the rest of the build system, and it has this same job each and every time it's invoked. Right now like any compiler, it assumes that any dependencies of the compilation have been installed and are ready to go.

ngcc, on the other hand, is intimately concerned with the build system. It has to deal with multiple package formats and dependencies between libraries. It has a vastly different lifecycle, and must understand when it has previously compiled packages and avoid recompiling them to save time. It also must understand the requirements of the overall bundling operation to know which package formats must be compiled and which are safe to ignore in the interest of speed.

Combining the two would require ngtsc, via its API, to become aware of much more detail about the current build process in order to run ngcc correctly. This would effectively transform ngtsc from a compiler into a build tool. I think that's a job that's better left to the real build toolchain, whether that's the CLI or Bazel or npm scripts.

I also disagree that running ngcc-on-demand via a hook in the ts.CompilerHost is the right approach for all users. This effectively serializes its operation, and I strongly believe there are major performance benefits to running ngcc in a multi-process mode, in parallel across the whole of node_modules. That's something we've yet to explore, but I think this would make ngcc as a post-install hook reasonably performant.

Need to update latest-versions.ts to beta.10 also?

@alexeagle It should not be needed as ~8.0.0-beta.9 does pick up beta.10

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.}