Skip to content

fix(docs-infra): fix redirecting rc.angular.io to angular.io when no active RC - #39853

Closed
gkalpak wants to merge 9 commits into
angular:masterfrom
gkalpak:build-aio-rc-redirects
Closed

gkalpak wants to merge 9 commits into
angular:masterfrom
gkalpak:build-aio-rc-redirects

Conversation

@gkalpak

@gkalpak gkalpak commented Nov 26, 2020 •

Copy link
Copy Markdown
Member

Currently there is an issue with redirecting rc.angular.io to angular.io when there is no active RC. If a user has visited rc.angular.io before and has a ServiceWorker registered for that subdomain, they will never "see" the redirect to angular.io.

This PR fixes the problem by doing an additional deployment from the stable branch to the rc-angular-io-site Firebase site when there is no active RC. This additional deployment will ensure that:

  1. Users will be temporarily redirected from rc.angular.io to angular.io.
  2. Users with a registered ServiceWorker (who don't see the redirect) will have their ServiceWorker unregistered on the next visit.
  3. The content on both sites is identical.

See #39760 for more details on the problem and the solution.

NOTE:
As mentioned in #39760, for this fix to take affect, we need to remove the redirect from rc.angular.io to angular.io in the Firebase console for site rc-angular-io-site.

Fixes #39760.

This PR also contains some other fixes and refactorings. See individual commits for more details.

@google-cla google-cla Bot added the cla: yes label Nov 26, 2020
@gkalpak gkalpak added comp: docs-infra refactoring Issue that involves refactoring or code-cleanup target: patch This PR is targeted for the next patch release type: bug/fix and removed cla: yes labels Nov 26, 2020
@ngbot ngbot Bot added this to the needsTriage milestone Nov 26, 2020
@google-cla google-cla Bot added the cla: yes label Nov 26, 2020
@ngbot ngbot Bot modified the milestone: needsTriage Nov 26, 2020
@google-cla

google-cla Bot commented Nov 26, 2020

Copy link
Copy Markdown

☹️ Sorry, but only Googlers may change the label cla: yes.

@ngbot ngbot Bot added this to the needsTriage milestone Nov 26, 2020
@mary-poppins

Copy link
Copy Markdown

You can preview f96d768 at https://pr39853-f96d768.ngbuilds.io/.

@gkalpak gkalpak added the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 26, 2020
@gkalpak
gkalpak marked this pull request as ready for review November 26, 2020 16:31
@gkalpak gkalpak changed the title fix(docs-infra): fix redirecting re.angular.io to angular.io when no active RC fix(docs-infra): fix redirecting rc.angular.io to angular.io when no active RC Nov 26, 2020
@gkalpak
gkalpak force-pushed the build-aio-rc-redirects branch from f96d768 to 4256ba5 Compare November 26, 2020 21:21
@mary-poppins

Copy link
Copy Markdown

You can preview 4256ba5 at https://pr39853-4256ba5.ngbuilds.io/.

@petebacondarwin petebacondarwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love love love the separate commits! This PR would have been a nightmare to review if it was all in one commit. Thank you for that.

A few minor comments but all in all LGTM.

Small typo (+query) for this commit message:

In fix(docs-infra): do not deploy as archive when major is not lower than stable

Typically, there will only be at most one minor branch mostmore recent than
the stable one (and that branch will be the RC branch).

(or the master branch?)

Comment thread aio/scripts/deploy-to-firebase.spec.js Outdated
Comment thread aio/scripts/deploy-to-firebase.js Outdated
Comment thread aio/scripts/deploy-to-firebase.spec.js Outdated
Comment thread aio/scripts/deploy-to-firebase.spec.js Outdated
Comment thread aio/scripts/deploy-to-firebase.js Outdated
Comment thread aio/scripts/deploy-to-firebase.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC why bck?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I believe there is a 62% chance in the future we will require to undo these modifications once deployed (for example, to deploy somewhere else). So, I am essentially keeping a backup to be able to restore the file later. In fact, there is an 11.2% chance a reviewer will request the clean-up be done in this PR 🤯

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And bck rather than bak?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good question 😅
I didn't think about it. I guess, that is a personal "convention" (because many programs create .bak files, I use .bck to avoid messing with other programs).

I'll change it to .bak to avoid confusion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: 30f8665

Comment thread aio/scripts/deploy-to-firebase.js Outdated
Comment thread aio/scripts/deploy-to-firebase.js Outdated
@gkalpak
gkalpak force-pushed the build-aio-rc-redirects branch from 4256ba5 to cff57f5 Compare November 28, 2020 12:04
@mary-poppins

Copy link
Copy Markdown

You can preview cff57f5 at https://pr39853-cff57f5.ngbuilds.io/.

… no active RC

Currently there is an issue with redirecting `rc.angular.io` to
`angular.io` when there is no active RC. If a user has visited
`rc.angular.io` before and has a ServiceWorker registered for that
subdomain, they will never "see" the redirect to `angular.io`.

This commit fixes the problem by doing an additional deployment from the
stable branch to the `rc-angular-io-site` Firebase site when there is no
active RC. This additional deployment will ensure that:
1. Users will be temporarily redirected from `rc.angular.io` to
   `angular.io`.
2. Users with a registered ServiceWorker (who don't see the redirect)
   will have their ServiceWorker unregistered on the next visit.
3. The content on both sites is identical.

See angular#39760 for more details on the problem and the solution.

NOTE:
As mentioned in angular#39760, for this fix to take affect, we need to remove
the redirect from `rc.angular.io` to `angular.io` in the Firebase
console for site `rc-angular-io-site`.

Fixes angular#39760
…scriptions

Previously, test descriptions used `latest` to refer to the most
recent/highest version. This was ambiguous, because `latest` can also
refer to the stable version of a package (e.g. see `@latest` npm tag).

This commit replaces `latest` with `highest` (or `highest for major`) to
reduce ambiguity.

Discussed in:
angular#39853 (comment)
@gkalpak
gkalpak force-pushed the build-aio-rc-redirects branch from 44c28d2 to 023529f Compare November 30, 2020 11:59
@mary-poppins

Copy link
Copy Markdown

You can preview 023529f at https://pr39853-023529f.ngbuilds.io/.

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 2, 2020
@mhevery mhevery closed this in 6e6eee6 Dec 2, 2020
mhevery pushed a commit that referenced this pull request Dec 2, 2020
Previously, the `deploy-to-firebase.js` script would only perform one
deployment operation on each run.

This commit adds support for performing multiple deployment operations.

NOTE:
In a subsequent commit, this will be leveraged fix an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker by
deploying the same artifacts to multiple Firebase projects/sites.
See #39760 for more details.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…ebase.spec.js` (#39853)

Previously, the latest commit for branch may be computed multiple times
in the `deploy-to-firebase.js` tests.

This commit avoids the unnecessary re-computations by computing the
latest commits for the necessary branches at the beginning and using the
computed values throughout the tests.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…han stable (#39853)

Previously, a branch would be deployed as `archive` even if it had a
major version that was equal/higher than that of the stable branch (as
long as it was not the RC branch - i.e. not the most recent minor
branch). For example, with `11.0.x` as the stable branch  and `12.0.x`
as the RC branch, `11.1.x` would be deployed as archive.

Theoretically, we should never find ourselves in such a situation.
Typically, there will only be at most one minor branch most recent than
the stable one (and that branch will be the RC branch). However, it
is possible under unusual circumstances.

This commit adds additional checks to guard against this problem. It
also refactors the code in preparation of fixing an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker, which will
require identifying whether there is an active RC version or not.
See #39760 for more details.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
… no active RC (#39853)

Currently there is an issue with redirecting `rc.angular.io` to
`angular.io` when there is no active RC. If a user has visited
`rc.angular.io` before and has a ServiceWorker registered for that
subdomain, they will never "see" the redirect to `angular.io`.

This commit fixes the problem by doing an additional deployment from the
stable branch to the `rc-angular-io-site` Firebase site when there is no
active RC. This additional deployment will ensure that:
1. Users will be temporarily redirected from `rc.angular.io` to
   `angular.io`.
2. Users with a registered ServiceWorker (who don't see the redirect)
   will have their ServiceWorker unregistered on the next visit.
3. The content on both sites is identical.

See #39760 for more details on the problem and the solution.

NOTE:
As mentioned in #39760, for this fix to take affect, we need to remove
the redirect from `rc.angular.io` to `angular.io` in the Firebase
console for site `rc-angular-io-site`.

Fixes #39760

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…scriptions (#39853)

Previously, test descriptions used `latest` to refer to the most
recent/highest version. This was ambiguous, because `latest` can also
refer to the stable version of a package (e.g. see `@latest` npm tag).

This commit replaces `latest` with `highest` (or `highest for major`) to
reduce ambiguity.

Discussed in:
#39853 (comment)

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…ploy-to-firebase.js` (#39853)

Previously, the `deploy()` function in `deploy-to-firebase.js` would
also perform other operations (beyond deploying), such as building the
app, checking the generated payload size, testing the PWA score of the
deployed app.

This commit decouples these operations, so that deploying can be
performed independently.

NOTE:
In a subsequent commit, this will be leveraged fix an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker by
deploying the same artifacts to multiple Firebase projects/sites.
See #39760 for more details.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
Previously, the `deploy-to-firebase.js` script would only perform one
deployment operation on each run.

This commit adds support for performing multiple deployment operations.

NOTE:
In a subsequent commit, this will be leveraged fix an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker by
deploying the same artifacts to multiple Firebase projects/sites.
See #39760 for more details.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…ebase.spec.js` (#39853)

Previously, the latest commit for branch may be computed multiple times
in the `deploy-to-firebase.js` tests.

This commit avoids the unnecessary re-computations by computing the
latest commits for the necessary branches at the beginning and using the
computed values throughout the tests.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…han stable (#39853)

Previously, a branch would be deployed as `archive` even if it had a
major version that was equal/higher than that of the stable branch (as
long as it was not the RC branch - i.e. not the most recent minor
branch). For example, with `11.0.x` as the stable branch  and `12.0.x`
as the RC branch, `11.1.x` would be deployed as archive.

Theoretically, we should never find ourselves in such a situation.
Typically, there will only be at most one minor branch most recent than
the stable one (and that branch will be the RC branch). However, it
is possible under unusual circumstances.

This commit adds additional checks to guard against this problem. It
also refactors the code in preparation of fixing an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker, which will
require identifying whether there is an active RC version or not.
See #39760 for more details.

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
… no active RC (#39853)

Currently there is an issue with redirecting `rc.angular.io` to
`angular.io` when there is no active RC. If a user has visited
`rc.angular.io` before and has a ServiceWorker registered for that
subdomain, they will never "see" the redirect to `angular.io`.

This commit fixes the problem by doing an additional deployment from the
stable branch to the `rc-angular-io-site` Firebase site when there is no
active RC. This additional deployment will ensure that:
1. Users will be temporarily redirected from `rc.angular.io` to
   `angular.io`.
2. Users with a registered ServiceWorker (who don't see the redirect)
   will have their ServiceWorker unregistered on the next visit.
3. The content on both sites is identical.

See #39760 for more details on the problem and the solution.

NOTE:
As mentioned in #39760, for this fix to take affect, we need to remove
the redirect from `rc.angular.io` to `angular.io` in the Firebase
console for site `rc-angular-io-site`.

Fixes #39760

PR Close #39853
mhevery pushed a commit that referenced this pull request Dec 2, 2020
…scriptions (#39853)

Previously, test descriptions used `latest` to refer to the most
recent/highest version. This was ambiguous, because `latest` can also
refer to the stable version of a package (e.g. see `@latest` npm tag).

This commit replaces `latest` with `highest` (or `highest for major`) to
reduce ambiguity.

Discussed in:
#39853 (comment)

PR Close #39853
@gkalpak
gkalpak deleted the build-aio-rc-redirects branch December 2, 2020 21:08
gkalpak added a commit to gkalpak/angular that referenced this pull request Dec 3, 2020
Since angular#39853, it is possible to deploy to multiple Firebase sites from a
single branch. In order to deploy to a site, we need to associate an
alias (`aio`) with a site. This is done via the `firebase target:apply`
command. However, when the command is called multiple times, it
associates the alias with many sites, which subsequently fails during
deployment ([example failure][1]), since the `firebase deploy` command
does not know what site to deploy to.

This commit fixes the deployment script by ensuring that any previous
association with the `aio` alias is cleared (via the
`firebase target:clear` command) before associating it with a new site.

[1]: https://circleci.com/gh/angular/angular/871020
mhevery pushed a commit that referenced this pull request Dec 3, 2020
Since #39853, it is possible to deploy to multiple Firebase sites from a
single branch. In order to deploy to a site, we need to associate an
alias (`aio`) with a site. This is done via the `firebase target:apply`
command. However, when the command is called multiple times, it
associates the alias with many sites, which subsequently fails during
deployment ([example failure][1]), since the `firebase deploy` command
does not know what site to deploy to.

This commit fixes the deployment script by ensuring that any previous
association with the `aio` alias is cleared (via the
`firebase target:clear` command) before associating it with a new site.

[1]: https://circleci.com/gh/angular/angular/871020

PR Close #39948
mhevery pushed a commit that referenced this pull request Dec 3, 2020
Since #39853, it is possible to deploy to multiple Firebase sites from a
single branch. In order to deploy to a site, we need to associate an
alias (`aio`) with a site. This is done via the `firebase target:apply`
command. However, when the command is called multiple times, it
associates the alias with many sites, which subsequently fails during
deployment ([example failure][1]), since the `firebase deploy` command
does not know what site to deploy to.

This commit fixes the deployment script by ensuring that any previous
association with the `aio` alias is cleared (via the
`firebase target:clear` command) before associating it with a new site.

[1]: https://circleci.com/gh/angular/angular/871020

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

Copy link
Copy Markdown

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 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes refactoring Issue that involves refactoring or code-cleanup target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(docs-infra) The SW interferes with redirects from rc.angular.io

4 participants