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

Broken routerLink on recompile when used with Ivy #32388

Closed
vajahath opened this issue Aug 29, 2019 · 3 comments

Comments

@vajahath
Copy link

@vajahath vajahath commented Aug 29, 2019

馃悶 bug report

<a routerLink="hiya">link</a>

The routerLink breaks when application recompiles. On browser, this is shown as an anchor tag that is not clickable.

This works correctly when Ivy is turned off.

Affected Package

I hope the issue may be related to @angular/router or may be Ivy

Description

I put the <a routerLink=".." .. in a component inside a lazyloaded module. When we stop the server and npm start again, this works correctly. But again fails on the next compile-on-save.

I turned off the Ivy flag and everything works fine. So this may be related to Ivy.

馃敩 Minimal Reproduction

I was following the lazy-loading-ngmodules guide.

A very minimal zip is attached here (without node_modules).
broken-routerLink.zip

馃敟 Exception or Error

No errors are shown.

馃實 Your Environment

Angular Version:


Angular CLI: 8.3.1
Node: 10.16.2
OS: linux x64
Angular: 8.2.4
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.803.1
@angular-devkit/build-angular     0.803.1
@angular-devkit/build-optimizer   0.803.1
@angular-devkit/build-webpack     0.803.1
@angular-devkit/core              8.3.1
@angular-devkit/schematics        8.3.1
@angular/cli                      8.3.1
@ngtools/webpack                  8.3.1
@schematics/angular               8.3.1
@schematics/update                0.803.1
rxjs                              6.4.0
typescript                        3.5.3
webpack                           4.39.2
@ngbot ngbot bot added this to the needsTriage milestone Aug 29, 2019
@kara kara added the comp: compiler label Aug 29, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Sep 5, 2019
@alxhub alxhub added this to Backlog in Compiler via automation Sep 5, 2019
@Kaffesumpen

This comment has been minimized.

Copy link

@Kaffesumpen Kaffesumpen commented Nov 2, 2019

Not sure if it matters but I am having the same issue.
it happens if I modify and re-save the template file.
If i use [routerLink] it will not recognise the attr
and if i type it like routerLink it will just ignore it
ng-reflect-router-link is not applied.

@IgorMinar IgorMinar modified the milestones: Backlog, v9-blockers Nov 9, 2019
@IgorMinar IgorMinar changed the title Broken routerLink when used with Ivy Broken routerLink on recompile when used with Ivy Nov 9, 2019
@mhevery mhevery self-assigned this Nov 11, 2019
@mhevery

This comment has been minimized.

Copy link
Member

@mhevery mhevery commented Nov 12, 2019

@alxhub I think this issue is for you.

The source code contains:

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    AppRoutingModule
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent {
  title = 'broken-routerLink';
}

The key thing to notice is templateUrl: './app.component.html'. The resulting generated code is:

        AppComponent.ngComponentDef = _angular_core__WEBPACK_IMPORTED_MODULE_0__["傻傻defineComponent"]({
            type: AppComponent,
            selectors: [["app-root"]],
            factory: function AppComponent_Factory(t) {
                return new (t || AppComponent)();
            },
            consts: 2,
            vars: 0,
            template: function AppComponent_Template(rf, ctx) {
                if (rf & 1) {
                    _angular_core__WEBPACK_IMPORTED_MODULE_0__["傻傻text"](0, "My App\n");
                    _angular_core__WEBPACK_IMPORTED_MODULE_0__["傻傻element"](1, "router-outlet");
                }
            },
            styles: ["\n/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IiIsImZpbGUiOiJzcmMvYXBwL2FwcC5jb21wb25lbnQuc2NzcyJ9 */"]
        });
        /*@__PURE__*/
        _angular_core__WEBPACK_IMPORTED_MODULE_0__["傻setClassMetadata"](AppComponent, [{
            type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Component"],
            args: [{
                selector: 'app-root',
                templateUrl: './app.component.html',
                styleUrls: ['./app.component.scss']
            }]
        }], null, null);

The key thing to notice is that defineComponent({ ... }) is missing directives

When I change AppComponent from templateUrl: './app.component.html' to template: '<router-outlet></router-outlet>' the generated output becomes correct and properly includes directives: [_angular_router__WEBPACK_IMPORTED_MODULE_1__["RouterOutlet"]],

So as far as I can tell the issue is that the compiler somehow gets confused with templateUrl.

@mhevery

This comment has been minimized.

Copy link
Member

@mhevery mhevery commented Nov 12, 2019

@alxhub says this may be fixed by #33551 or #33522.

@mhevery mhevery assigned mhevery and unassigned mhevery Nov 13, 2019
alxhub added a commit to alxhub/angular that referenced this issue Nov 15, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
alxhub added a commit to alxhub/angular that referenced this issue Nov 15, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
alxhub added a commit to alxhub/angular that referenced this issue Nov 16, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
Fixes angular#32213
alxhub added a commit to alxhub/angular that referenced this issue Nov 16, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
alxhub added a commit to alxhub/angular that referenced this issue Nov 18, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
alxhub added a commit to alxhub/angular that referenced this issue Nov 19, 2019
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes angular#32388
Fixes angular#32214
alxhub added a commit that referenced this issue Nov 20, 2019
鈥33862)

Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes #32388
Fixes #32214

PR Close #33862
@alxhub alxhub closed this in 4be8929 Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.