Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility Compiler Fails When Dependency has Multiple Modules with Same Named Pipe #33593

Closed
sourdoughdetzel opened this issue Nov 5, 2019 · 3 comments

Comments

@sourdoughdetzel
Copy link

@sourdoughdetzel sourdoughdetzel commented Nov 5, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/compiler

Is this a regression?

Unfortunately, I am not sure. My first attempt at using Ivy has been with `next.15` and `rc.0`. I have experienced the issue in both.

Description

When an app pulls in a node module that has multiple modules in it, and those modules define their own pipes that have the same name, `ngcc` fails. Ex. `CoolPackage` has `ModuleA` with pipe called `CoolPipe` and `ModuleB` with pipe also called `CoolPipe`.

馃敩 Minimal Reproduction

Please pull down the repo here and follow the README instructions.

馃敟 Exception or Error

From our live app:





ERROR in ../node_modules/@local/angular-pattern-library/multi-select-inline-search/multi-select-inline-search.module.d.ts:3:25 - error TS7016: Could not find a declaration file for module '../fesm2015/angular-pattern-library'. 'C:/app-path/node_modules/@local/angular-pattern-library/fesm2015/angular-pattern-library.js' implicitly has an 'any' type.

3 import * as 傻ngcc2 from '../fesm2015/angular-pattern-library';

From the demo app above:


ERROR in Cannot resolve type entity 傻ngcc2.傻b to symbol

馃實 Your Environment

Angular Version:




Angular CLI: 9.0.0-rc.0
Node: 10.16.0
OS: win32 x64
Angular: 9.0.0-rc.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.0
@angular-devkit/build-angular     0.900.0-rc.0
@angular-devkit/build-optimizer   0.900.0-rc.0
@angular-devkit/build-webpack     0.900.0-rc.0
@angular-devkit/core              9.0.0-rc.0
@angular-devkit/schematics        9.0.0-rc.0
@angular/http                     7.2.15
@ngtools/webpack                  9.0.0-rc.0
@schematics/angular               9.0.0-rc.0
@schematics/update                0.900.0-rc.0
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

Anything else relevant?

Although it may be the case that the pipes probably shouldn't have the same exact name, the fact that they are contained to the modules would make me think it shouldn't matter.

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 4, 2019

I have been looking into this. The problem primarily arises from the fact that the package has a flat ESM2015 format (i.e. all the JS source squashed into one file) while the typings is a non-flat format (i.e. a tree of .d.ts files).

In the flat ESM2015 format the second TestPipePipe is renamed to TestPipePipe$1 to prevent collisions with TestPipePipe, and then they are exported as "private exports": TestPipePipe as 傻a and TestPipePipe$1 as 傻b.

In the typings file tree there is no reference at all to TestPipePipe$1.

When ngcc does its processing it must update the .d.ts files with extra typing information about the types of the declarations of NgModules. For example the TestOneModule typings file goes from:

export declare class TestOneModule {
}

to

import * asngcc0 from '@angular/core';
import * asngcc1 from './test-one.component';
import * asngcc2 from './test-pipe.pipe';
import * asngcc3 from '@angular/common';
export declare class TestOneModule {
    static 傻mod:ngcc0.傻傻NgModuleDefWithMeta<TestOneModule, [typeofngcc1.TestOneComponent, typeofngcc2.TestPipePipe], [typeofngcc3.CommonModule], never>;
    static 傻inj:ngcc0.傻傻InjectorDef<TestOneModule>;
}

If the JS format has the same layout (e.g. non-flat) as the typings files then it is possible to match the names of the types in the JS files to the names of the types in the typings files quite accurately.

If the formats are different (e.g. JS is flat and typings are non-flat) then it has to do its best by matching the name of the identifier from the JS (e.g. TestPipePipe in the first module and TestPipePipe$1 in the second module) to a name found in the typings file tree.

In this case there is no mention of TestPipePipe$1 in the typings tree and so it is not possible to match.

Ngcc does its best because it knows that in the JS this identifier is exported as 傻b and so tries to create an imported type for that - import * as 傻ngcc2 from '../fesm2015/test-package'; - but actually this is completely wrong since the typings never exported this symbol either (note the import from the fesm) and so we should error out at this point of ngcc's processing (or at least log a warning). The resulting typings file for TestTwoModule looks like:

import * asngcc0 from '@angular/core';
import * asngcc1 from './test-two.component';
import * asngcc2 from '../fesm2015/test-package';
import * asngcc3 from '@angular/common';
export declare class TestTwoModule {
    static 傻mod:ngcc0.傻傻NgModuleDefWithMeta<TestTwoModule, [typeofngcc1.TestTwoComponent, typeofngcc2.傻b], [typeofngcc3.CommonModule], never>;
    static 傻inj:ngcc0.傻傻InjectorDef<TestTwoModule>;
}
@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 4, 2019

I will attempt a fix in ngcc...

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 5, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 7, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 7, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 11, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 11, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 11, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 11, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 12, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 18, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 18, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.

Fixes angular#33593
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 18, 2019
The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.

Fixes angular#33593
@kara kara closed this in f22a6eb Dec 18, 2019
kara added a commit that referenced this issue Dec 18, 2019
鈥34254)

The na茂ve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.

Fixes #33593

PR Close #34254
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Jan 18, 2020

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.