fix(upgrade): fix AngularJsUrlCodec to support Safari

[agale123]

Safari throws an error when the new URL() constructor is called with an
undefined base. This change checks whether the base is undefined and
then calls the corresponding version of the URL constructor.

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

Issue Number: #32874

What is the new behavior?

The AngularJsUrlCodec used by the $locationShim works in Safari.

Does this PR introduce a breaking change?

• Yes
• No

Other information

[petebacondarwin]

Thanks for this PR @agale123!
I think the fix could be simpler but otherwise LGTM.

[trotyl]

I don't think this is the right approach, URL constructor is part of the core-js polyfills, if Angular made use of it, then it should be a mandatory polyfill in Angular CLI, rather than asking every package to workaround it individually.

EDIT: May better be commented with notes like enable this when you use @angular/upgrade.

[petebacondarwin]

@trotyl - I don't think that including this polyfill would solve the problem, unless I misunderstand how the polyfill would be applied. Safari supports URL and so my understanding is that the polyfill would not be applied. In which case we still have the problem.

[petebacondarwin]

Or perhaps I am wrong and that applying the polyfill will always override the native URL? In which case that seems overkill for such a simple fix?

[trotyl]

@petebacondarwin Core-js has a built-in feature testing for it to check whether it works well: https://github.com/zloirock/core-js/blob/ba63aec37db946081b40148521bdf27188c3f258/packages/core-js/internals/native-url.js#L7-L24

[petebacondarwin]

There is no check for new URL('abc', undefined); in those tests though... 🤔

[trotyl]

There is no check for new URL('abc', undefined); in those tests though... 🤔

@petebacondarwin It should be covered by all the single parameter cases like new URL('https://a@b').username !== 'a', if not supported this will throw an error and fail the whole test.

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[petebacondarwin]

It should be covered by all the single parameter cases like new URL('https://a@b').username !== 'a',...

This is not the issue, passing only one parameter works. In fact this was @agale123 's original fix.
The problem is if you explicitly pass undefined as the 2nd argument.

[petebacondarwin]

@agale123 could you rebase and squash all the commits into one please?

[agale123]

@agale123 could you rebase and squash all the commits into one please?

Done.

[gkalpak]

Thx, @agale123! One comment about the implementation, but the direction is right 😁
_{BTW, it would be great if we could add some tests for parse() (but not blocking since we didn't have explicit tests before).}

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[agale123]

ct

That's good to know, I'll go ahead and make that change.

[trotyl]

@petebacondarwin Raised zloirock/core-js#656 for core-js issue tracking.

[atscott]

Presubmit

[angular-automatic-lock-bot]

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.}