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

ngc --watch ignores changes in templates (ivy, 9.0.0-next.8) #32869

Open
stefanocke opened this issue Sep 26, 2019 · 14 comments 路 May be fixed by #34015

Comments

@stefanocke
Copy link

@stefanocke stefanocke commented Sep 26, 2019

This is a repost of #31982, since:

  • the bug is still there in 9.0.0-next.8 and 8.2.8 ,
  • it is reproducible (as before) with the given example and
  • it has been closed unjustified as duplicate by @alxhub.

馃悶 bug report
Affected Package
The issue is caused by package @angular/compiler-cli

Is this a regression?
Yes, the previous version in which this bug was not present was: 8.1.2
(But, due to #31322, it only worked on every second change of a template.)

Description
I run ngc in watch mode. Output directory is out-tsc.
Ivy is enabled.
I change a component's template.
ngc says "File change detected. Starting incremental compilation."
However, the template change does not result in an according change in the *.js file in out-tsc directory.
For changes in *.ts files, it works.

馃敩 Minimal Reproduction
Example project is at: https://github.com/stefanocke/angular-cli-app-with-rollup
Please run:

npm i
npm run ngc-app-watch

(This is nothing more than running ngc in watch mode, as the title of this bug says.)

Then, change app.component.html and look for app.component.js in tsc-out.

You will see that, although the console says correctly "File change detected. Starting incremental compilation.", nothing has changed in app.component.js.

馃實 Your Environment
Angular Version:

Angular CLI: 8.3.6
Node: 10.16.0
OS: win32 x64
Angular: 9.0.0-next.8
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, router

Package Version

@angular-devkit/architect 0.900.0-next.6
@angular-devkit/build-angular 0.900.0-next.6
@angular-devkit/build-optimizer 0.900.0-next.6
@angular-devkit/build-webpack 0.900.0-next.6
@angular-devkit/core 9.0.0-next.6
@angular-devkit/schematics 8.3.6
@angular/cli 8.3.6
@ngtools/webpack 9.0.0-next.6
@schematics/angular 8.3.6
@schematics/update 0.803.6
rxjs 6.5.2
typescript 3.5.3

Anything else relevant?
Ivy is enabled

@ngbot ngbot bot modified the milestone: needsTriage Sep 26, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Oct 8, 2019
gabrielaraujof added a commit to gabrielaraujof/dog-hero that referenced this issue Oct 19, 2019
The changes on html aren't being affected by the serve build.
For more details please visit the open (at the time of this commit)
issue: angular/angular#32869
JoostK added a commit to JoostK/angular that referenced this issue Nov 2, 2019
When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.

There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.

Fixes angular#32869
@JoostK JoostK added the state: has PR label Nov 2, 2019
atscott added a commit that referenced this issue Nov 7, 2019
鈥#33551)

When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.

There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.

Fixes #32869

PR Close #33551
@atscott atscott closed this in 8912b11 Nov 7, 2019
@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 7, 2019

@stefanocke just FYI the fix had to be reverted because of an issue with running the tests on Windows. I will follow up with a fix for that so that we can get it landed again. Sorry for the inconvenience.

@JoostK JoostK reopened this Nov 7, 2019
@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 9, 2019

@JoostK , thanks for looking into this, nevertheless.

JoostK added a commit to JoostK/angular that referenced this issue Nov 9, 2019
When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.

There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.

Fixes angular#32869
kara added a commit that referenced this issue Nov 12, 2019
鈥#33551)

When the Angular compiler is operated through the ngc binary in watch
mode, changing a template in an external file would not cause the
component to be recompiled if Ivy is enabled.

There was a problem with how a cached compiler host was present that was
unaware of the changed resources, therefore failing to trigger a
recompilation of a component whenever its template changes. This commit
fixes the issue by ensuring that information about modified resources is
correctly available to the cached compiler host.

Fixes #32869

PR Close #33551
@kara kara closed this in 6899ee5 Nov 12, 2019
@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

@kara , @JoostK : unfortunately it still does not work for me. I just upgraded the example project to 9.0.0-rc.2 ...

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 15, 2019

@stefanocke :( Let me check to see what's going on. I did test the PR for your repro, will have another go using rc.2 (I see that you already updated in the repro repo, thanks!)

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 15, 2019

@stefanocke With your latest commit using rc.2 I cannot reproduce. Could it be that I'm missing something? If I edit just a single template I do see the corresponding .js file being updated with the template changes applied.

@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

@JoostK thank you, I will try again. Maybe I am missing something myself...

@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

@JoostK , still no success. I deleted node_modules and started freshly.

Would you mind to give me some details of your successful test?

  • Linux or Windows?
  • which command / npm script did you use?
  • which template did you change and which *.js file did you check?

(I know, most of that is obvious. But just to see if there is subtle different detail...)

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 15, 2019

@JoostK , still no success. I deleted node_modules and started freshly.

Would you mind to give me some details of your successful test?

  • Linux or Windows?

OS X using Node 12

  • which command / npm script did you use?

npm run ngc-app-watch

  • which template did you change and which *.js file did you check?

Mostly src/app/app.component.html by adding some text after the Welcome to {{ title }}!, then I check out-tsc/src/app/app.component.js. I have also deleted app-comp2 through app-comp20, which is also refreshed correctly, then re-adding them back also works correctly for me. Additionally have I changed some text in src/app/ballast/comp1/comp1.component.html, which is also correctly updated in the compiled .js. Before the fix all these scenarios were not working for me as well, but now with rc.2 I don't have any more issues.

(I know, most of that is obvious. But just to see if there is subtle different detail...)

No worries. Hopefully you spot something different. Let me know what else I can try :-)

@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

This new part of the code results in C:\\{some-path-on-my-machine}\\src\\app\\app.component.html for me, which seems to be correct:

// Provide access to the file paths that triggered this rebuild
                cachedCompilerHost.getModifiedResourceFiles = function () {
                    if (timerHandleForRecompilation === undefined) {
                        return undefined;
                    }
                    //debug
                    console.log(timerHandleForRecompilation.modifiedResourceFiles);
                    return timerHandleForRecompilation.modifiedResourceFiles;
                };
@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

Okay, if I add C:/{some-path-on-my-machine}/src/app/app.component.html to the modifiedResourceFiles, it works!
That is, it is still an Windows related issue with separator "\" instead of "/".

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 15, 2019

Can you place strategic breakpoints in these locations:

const newFiles = new Set<ts.SourceFile>(newProgram.getSourceFiles());

if (timerHandleForRecompilation === undefined) {

and possible here as well:

if (this.modifiedResourceFiles === null || !this.metadata.has(sf)) {

@JoostK

This comment has been minimized.

Copy link
Member

@JoostK JoostK commented Nov 15, 2019

Okay, if I add C:/{some-path-on-my-machine}/src/app/app.component.html to the modifiedResourceFiles, it works!
That is, it is still an Windows related issue with separator "" instead of "/".

Hmm, thanks for finding the culprit. This is surprising to me, as the test I added is also run on Windows (we had to revert it initially because it wasn't working). I'll have a look!

@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 15, 2019

Thank you again. Since it somehow get lost during markup formatting, I would like to emphasize that I replaced double-backslash by slash in the path.

@stefanocke

This comment has been minimized.

Copy link
Author

@stefanocke stefanocke commented Nov 16, 2019

@JoostK , could you please re-open. I cannot...

@JoostK JoostK reopened this Nov 16, 2019
JoostK added a commit to JoostK/angular that referenced this issue Nov 23, 2019
In angular#33551, a bug in `ngc --watch` mode was fixed so that a component is
recompiled when its template file is changed. Due to insufficient
normalization of files paths, this fix did not have the desired effect
on Windows.

Fixes angular#32869
JoostK added a commit to JoostK/angular that referenced this issue Nov 24, 2019
In angular#33551, a bug in `ngc --watch` mode was fixed so that a component is
recompiled when its template file is changed. Due to insufficient
normalization of files paths, this fix did not have the desired effect
on Windows.

Fixes angular#32869
JoostK added a commit to JoostK/angular that referenced this issue Nov 24, 2019
In angular#33551, a bug in `ngc --watch` mode was fixed so that a component is
recompiled when its template file is changed. Due to insufficient
normalization of files paths, this fix did not have the desired effect
on Windows.

Fixes angular#32869
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.