ngcc: doesn't process library prefixed exports properly

[matheo]

🐞 bug report

Affected Package

The issue is caused by package @angular/compiler-cli, package where ngcc belongs.

Is this a regression?

No, this is the first time we're trying to upgrade from Angular 8 to 9.

Description

We have 30+ Angular Libraries released with Angular v8 right now.
Upgrading the websites from v8 to v9, ngcc throws exceptions from the compiled files of our libraries (__ivy_ngcc__/fesm2015/app.js). The exceptions are related to Injectables consumed from our /common library.

The issue is that we export our Services with the company prefix,
and ngcc is not compiling the libraries properly missing such prefix and not finding the Service.
I will go deep in the exception to illustrate the problem.

🔥 Exception or Error

ERROR in ./node_modules/@company/services/__ivy_ngcc__/fesm2015/app.js 7920:148-166
"export 'HttpService' (imported as 'ɵngcc1') was not found in '@company/common'

ERROR in ./node_modules/@company/forms/__ivy_ngcc__/fesm2015/app.js 11190:272-294
"export 'PhoneNumberPipe' (imported as 'ɵngcc3') was not found in '@company/common'

Where node_modules/@company/services/__ivy_ngcc__/fesm2015/app.js has:

import * as ɵngcc0 from '@angular/core';
import * as ɵngcc1 from '@myndmanagement/common';

(function () { ɵngcc0.ɵsetClassMetadata(ChargeCodeService, [{
        type: Injectable
    }], function () { return [{ type: ɵngcc1.HttpService }]; }, null); })();

but ɵngcc1.HttpService does not exists, it should be PrefixHttpService.

Looking at node_modules/@company/services/fesm2015/app.js it seems ok:

import { PrefixHttpService } from '@company/common';
...
ChargeCodeService.decorators = [
    { type: Injectable }
];
/** @nocollapse */
ChargeCodeService.ctorParameters = () => [
    { type: MyndHttpService }
];
...
export { ChargeCodeService as PrefixChargeCodeService };

Then Ivy's compiler is removing the Prefix for some reason :(
the library's index.d.ts correctly maps them as:

export {
  CommandsService as PrefixCommandsService,
  ...,
  HttpService as PrefixHttpService,
  ...,
} from './services/index';

🔬 Minimal Reproduction

I will provide enough details to show the problem, as a reproduction is complicated right now.
I can mount a reproduction the next weekend if it's required.

🌍 Your Environment

Angular Version:

Angular CLI: 9.1.12
Node: 10.18.0
OS: linux x64

Angular: 9.1.12
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router, service-worker
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.12
@angular-devkit/build-angular     0.901.12
@angular-devkit/build-optimizer   0.901.12
@angular-devkit/build-webpack     0.901.12
@angular-devkit/core              9.1.12
@angular-devkit/schematics        9.1.12
@angular/cdk                      9.2.4
@angular/flex-layout              9.0.0-beta.31
@angular/pwa                      0.901.12
@ngtools/webpack                  9.1.12
@schematics/angular               9.1.12
@schematics/update                0.901.12
rxjs                              6.5.5
typescript                        3.8.3
webpack                           4.42.0

Anything else relevant?

@gkalpak I've seen some PRs from you fixing re-exports, and this topic seems related to your work.
Could you give us a hand here please? thanks in advance!!

[JoostK]

I can reproduce locally in a small setup. Compiling the libraries directly for Ivy does not show the issue for me, so it does indeed appear to be specific to ngcc.

[matheo]

@JoostK I've tried to compile them with different tsconfig options with no joy.

Our way to go at the moment will be to keep the Libraries in Angular 8
and proceed to upgrade the Websites to Angular 9 with Ivy disabled until a solution comes up.
Let me know if I can be of any help please, and thanks in advance! :)

[gkalpak]

A little update:
The problem is indeed in ngcc, more specifically in the Esm2015ReflectionHost#getConstructorParamInfo(), where we use the declaration's identifier as the importedName (ignoring the fact that something can be exported with a different name):

angular/packages/compiler-cli/ngcc/src/host/esm2015_host.ts

Lines 1605 to 1611 in 8fbf40b

	typeValueReference = {
	local: false,
	valueDeclaration: decl.node,
	moduleName: decl.viaModule,
	importedName: decl.node.name.text,
	nestedPath: null,
	};

I am still not sure what is the best way to fix it. Annoyingly, I haven't been able to correctly reproduce this in a unit test in esm2015_host_spec.ts yet 😢

Example failing test

// ...
describe('getConstructorParameters()', () => {
  // ...
  it('should find the decorated constructor parameters', () => {
    const files = [
      {
        name: _('/node_modules/shared-lib/package.json'),
        contents: `
          {
            "name": "shared-lib",
            "es2015": "./index.js",
            "types": "./index.d.ts"
          }
        `,
      },
      {
        name: _('/node_modules/shared-lib/foo.d.ts'),
        contents: `
          declare class Foo {
          }

          export Foo as Bar;
        `,
      },
      {
        name: _('/node_modules/shared-lib/foo.js'),
        contents: `
          class Foo {
          }
 
          export Foo as Bar;
        `,
      },
      {
        name: _('/node_modules/shared-lib/index.d.ts'),
        contents: `
          export {Bar as Baz} from './foo';
        `,
      },
      {
        name: _('/node_modules/shared-lib/index.js'),
        contents: `
          export {Bar as Baz} from './foo';
        `,
      },
      {
        name: _('/main.d.ts'),
        contents: `
          import {Baz} from 'shared-lib/index';

          export declare class SomeClass {
            constructor(arg1: Baz) {
            }
          }
        `,
      },
      {
        name: _('/main.js'),
        contents: `
          import {Baz} from 'shared-lib/index';

          export class SomeClass {
            constructor(arg1) {
            }
          }
          SomeClass.ctorParameters = [{ type: Baz }];
        `,
      },
    ];

    loadFakeCore(getFileSystem());
    loadTestFiles(files);

    const bundle = makeTestBundleProgram(_('/main.js'));
    const host = createHost(bundle, new Esm2015ReflectionHost(new MockLogger(), false, bundle));
    const classNode = getDeclaration(
        bundle.program, _('/main.js'), 'SomeClass', isNamedClassDeclaration);
    const parameters = host.getConstructorParameters(classNode)!;

    expect(parameters).toEqual([jasmine.objectContaining({name: 'arg1'})]);
    expectTypeValueReferencesForParameters(parameters, ['Qux'], 'shared-lib');
  });
});

I would expect that the *.js files would be enough to reproduce the problem, but added the package.json and *.d.ts files just in case.

[matheo]

wow, amazing tracking of the issue tho.
unfortunately I haven't played with this kind of stuff before,
but if I get the time I will try to run the test and give it a shot by myself
thanks for your work @gkalpak!

[petebacondarwin]

@gkalpak your failing test was almost perfect. There was just a minor typo in the export aliases:

class Foo {}
export Foo as Bar;

should be

class Foo {}
export {Foo as Bar};

Now (for me) it is failing (as expected) with

Expected 'shared-lib/index' to be 'shared-lib'.

and

Expected 'Foo' to be 'Qux'.

(although I think we actually mean Baz not Qux)

[gkalpak]

🙇
Thank you so much, @petebacondarwin ❤️ ❤️ ❤️
This has cost me several hours of head-scratching before I decide to move to something else and come back to it later.

_{Good timing too - I was planning to get back to it today or tomorrow.}

[hixio-mh]

Duplicate of #
Merge pull requested

[angular-automatic-lock-bot]

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