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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): ensure module scope is rebuild on dependent change #33522

Closed
wants to merge 1 commit into from

Conversation

@JoostK
Copy link
Member

JoostK commented Oct 31, 2019

During incremental compilations, ngtsc needs to know which metadata
from a previous compilation can be reused, versus which metadata has to
be recomputed as some dependency was updated. Changes to
directives/components should cause the NgModule in which they are
declared to be recompiled, as the NgModule's compilation is dependent
on its directives/components.

When a dependent source file of a directive/component is updated,
however, a more subtle dependency should also cause to NgModule's source
file to be invalidated. During the reconciliation of state from a
previous compilation into the new program, the component's source file
is invalidated because one of its dependency has changed, ergo the
NgModule needs to be invalidated as well. Up until now, this implicit
dependency was not imposed on the NgModule. Additionally, any change to
a dependent file may influence the module scope to change, so all
components within the module must be invalidated as well.

This commit fixes the bug by introducing additional file dependencies,
as to ensure a proper rebuild of the module scope and its components.

Fixes #32416

During incremental compilations, ngtsc needs to know which metadata
from a previous compilation can be reused, versus which metadata has to
be recomputed as some dependency was updated. Changes to
directives/components should cause the NgModule in which they are
declared to be recompiled, as the NgModule's compilation is dependent
on its directives/components.

When a dependent source file of a directive/component is updated,
however, a more subtle dependency should also cause to NgModule's source
file to be invalidated. During the reconciliation of state from a
previous compilation into the new program, the component's source file
is invalidated because one of its dependency has changed, ergo the
NgModule needs to be invalidated as well. Up until now, this implicit
dependency was not imposed on the NgModule. Additionally, any change to
a dependent file may influence the module scope to change, so all
components within the module must be invalidated as well.

This commit fixes the bug by introducing additional file dependencies,
as to ensure a proper rebuild of the module scope and its components.

Fixes #32416
@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Nov 6, 2019

Presubmit
No Ivy presubmit needed; incremental rebuild wouldn't be tested correctly anyway.

@ngbot

This comment has been minimized.

Copy link

ngbot bot commented Nov 12, 2019

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "google3" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@JoostK

This comment has been minimized.

Copy link
Member Author

JoostK commented Nov 12, 2019

merge-assistance: merge label was removed, I don't know why? It should be good to go from my perspective.

@kara

This comment has been minimized.

Copy link
Contributor

kara commented Nov 12, 2019

@JoostK Looks like the PR still needs a review

@alxhub
alxhub approved these changes Nov 12, 2019
kara added a commit that referenced this pull request Nov 12, 2019
During incremental compilations, ngtsc needs to know which metadata
from a previous compilation can be reused, versus which metadata has to
be recomputed as some dependency was updated. Changes to
directives/components should cause the NgModule in which they are
declared to be recompiled, as the NgModule's compilation is dependent
on its directives/components.

When a dependent source file of a directive/component is updated,
however, a more subtle dependency should also cause to NgModule's source
file to be invalidated. During the reconciliation of state from a
previous compilation into the new program, the component's source file
is invalidated because one of its dependency has changed, ergo the
NgModule needs to be invalidated as well. Up until now, this implicit
dependency was not imposed on the NgModule. Additionally, any change to
a dependent file may influence the module scope to change, so all
components within the module must be invalidated as well.

This commit fixes the bug by introducing additional file dependencies,
as to ensure a proper rebuild of the module scope and its components.

Fixes #32416

PR Close #33522
@kara kara closed this in 15f8638 Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.