fix(service-worker): make `ngsw.json` generation deterministic and correct by gkalpak · Pull Request #43679 · angular/angular

@gkalpak gkalpak marked this pull request as ready for review

October 2, 2021 15:27

gkalpak added a commit to gkalpak/angular that referenced this pull request

Oct 3, 2021

petebacondarwin

petebacondarwin

dylhunn pushed a commit that referenced this pull request

Oct 5, 2021
…ministic (#43687)

As explained in #43679, currently the generation of the `ngsw.json` SW
manifest is non-deterministic and often incorrect. Until we can update
to an `@angular/service-worker` version that includes the fix from
PR #43679, we temporarily work-around the issue by re-generating the
`ngsw.json` manifest after `ng build` using the `ngsw-config` binary
exposed by `@angular/service-worker`.

NOTE:
This works around the issue, because the [FileSystem][1] class used by
the `ngsw-config` binary happens to be synchronous (unlike the
implementation provided by the Angular CLI), thus avoiding the race
conditions described in #43679.

[1]: https://github.com/angular/angular/blob/c721135e370b34c840756bcfb22c8119b4c8c452/packages/service-worker/cli/filesystem.ts#L15

PR Close #43687

@gkalpak gkalpak removed the action: review

The PR is still awaiting reviews from at least one requested reviewer

label

Oct 5, 2021
…rrect

Previously, all asset-groups from `ngsw-config.json` were processed in
parallel. For each asset-group, we retrieved all files for the current
build, filtered out files that were already matched by other
asset-groups, determined which of the remaining files belonged to the
current asset-group and generated entries for the `ngsw.json` manifest.
This process was susceptible to race conditions when there were files
that would be matched by multiple asset-groups. This made the generation
of the `ngsw.json` manifest non-deterministic and violated the rule that
each file would belong to the first asset-group that matched it (based
on the asset-groups' order of appearance in `ngsw-config.json`), thus
leading to broken ServiceWorker behavior.

This commit fixes it by ensuring that the generation process is
deterministic and that asset-groups are processed in the proper order.

NOTE 1:
The generation process has been broken since the beginning, but we have
only noticed this recently. This is possibly related to the CLI's
switching from a virtual file system host (which has more consistent
timing characteristics) to the Node.js built-in `fs.promises` in
angular/angular-cli@d3bc530.

NOTE 2:
This commit also ensures that files in the `ngsw.json` hash-table are in
alphabetic order. Previously, the files were added to the hash-table in
blocks corresponding to each asset-group.
This change is not necessary (i.e. the order of keys in the hash-table
makes no difference in behavior), but it makes it easier to scan for a
file (for example, for debugging purposes).

@gkalpak

dylhunn pushed a commit that referenced this pull request

Oct 5, 2021
…rrect (#43679)

Previously, all asset-groups from `ngsw-config.json` were processed in
parallel. For each asset-group, we retrieved all files for the current
build, filtered out files that were already matched by other
asset-groups, determined which of the remaining files belonged to the
current asset-group and generated entries for the `ngsw.json` manifest.
This process was susceptible to race conditions when there were files
that would be matched by multiple asset-groups. This made the generation
of the `ngsw.json` manifest non-deterministic and violated the rule that
each file would belong to the first asset-group that matched it (based
on the asset-groups' order of appearance in `ngsw-config.json`), thus
leading to broken ServiceWorker behavior.

This commit fixes it by ensuring that the generation process is
deterministic and that asset-groups are processed in the proper order.

NOTE 1:
The generation process has been broken since the beginning, but we have
only noticed this recently. This is possibly related to the CLI's
switching from a virtual file system host (which has more consistent
timing characteristics) to the Node.js built-in `fs.promises` in
angular/angular-cli@d3bc530.

NOTE 2:
This commit also ensures that files in the `ngsw.json` hash-table are in
alphabetic order. Previously, the files were added to the hash-table in
blocks corresponding to each asset-group.
This change is not necessary (i.e. the order of keys in the hash-table
makes no difference in behavior), but it makes it easier to scan for a
file (for example, for debugging purposes).

PR Close #43679

@gkalpak gkalpak deleted the fix-sw-config-asset-race branch

October 6, 2021 13:10

gkalpak added a commit to gkalpak/angular that referenced this pull request

Oct 6, 2021

dylhunn pushed a commit that referenced this pull request

Oct 6, 2021
…ministic (#43686)

As explained in #43679, currently the generation of the `ngsw.json` SW
manifest is non-deterministic and often incorrect. Until we can update
to an `@angular/service-worker` version that includes the fix from
PR #43679, we temporarily work-around the issue by re-generating the
`ngsw.json` manifest after `ng build` using the `ngsw-config` binary
exposed by `@angular/service-worker`.

NOTE:
This works around the issue, because the [FileSystem][1] class used by
the `ngsw-config` binary happens to be synchronous (unlike the
implementation provided by the Angular CLI), thus avoiding the race
conditions described in #43679.

[1]: https://github.com/angular/angular/blob/c721135e370b34c840756bcfb22c8119b4c8c452/packages/service-worker/cli/filesystem.ts#L15

PR Close #43686

Serginho pushed a commit to TuLotero/angular that referenced this pull request

Jan 20, 2022
…rrect (angular#43679)

Previously, all asset-groups from `ngsw-config.json` were processed in
parallel. For each asset-group, we retrieved all files for the current
build, filtered out files that were already matched by other
asset-groups, determined which of the remaining files belonged to the
current asset-group and generated entries for the `ngsw.json` manifest.
This process was susceptible to race conditions when there were files
that would be matched by multiple asset-groups. This made the generation
of the `ngsw.json` manifest non-deterministic and violated the rule that
each file would belong to the first asset-group that matched it (based
on the asset-groups' order of appearance in `ngsw-config.json`), thus
leading to broken ServiceWorker behavior.

This commit fixes it by ensuring that the generation process is
deterministic and that asset-groups are processed in the proper order.

NOTE 1:
The generation process has been broken since the beginning, but we have
only noticed this recently. This is possibly related to the CLI's
switching from a virtual file system host (which has more consistent
timing characteristics) to the Node.js built-in `fs.promises` in
angular/angular-cli@d3bc530.

NOTE 2:
This commit also ensures that files in the `ngsw.json` hash-table are in
alphabetic order. Previously, the files were added to the hash-table in
blocks corresponding to each asset-group.
This change is not necessary (i.e. the order of keys in the hash-table
makes no difference in behavior), but it makes it easier to scan for a
file (for example, for debugging purposes).

PR Close angular#43679