Skip to content

Use globalThis for global #50063

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.

Sign up for GitHub

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

Jump to bottom
Closed
JeanMeche wants to merge 4 commits into from
Closed

Use globalThis for global #50063

JeanMeche wants to merge 4 commits into from

Conversation

@JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Apr 28, 2023 •
edited
Loading

use globalThis for global

globalThis is now available on every runtime supported by Angular

See individual commits for the 3 impacted packages.

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • No

Other information

@JeanMeche JeanMeche marked this pull request as ready for review April 28, 2023 12:21
@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime area: compiler Issues related to `ngc`, Angular's template compiler labels Apr 28, 2023
@ngbot ngbot bot modified the milestone: Backlog Apr 28, 2023
@pkozlowski-opensource pkozlowski-opensource added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 28, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Jun 10, 2023

I have no idea whether this is correct, and I'm honest not sure who to ask. Maybe @devversion?

@alfaproject
Copy link
Contributor

alfaproject commented Jun 10, 2023

Isn't globalThis supported by all the browsers that Angular supports now?

@JeanMeche
Copy link
Member Author

JeanMeche commented Jun 10, 2023

@alfaproject You're making a goodpoint, the global definitions could be simplified now that IE has been dropped.

@JeanMeche JeanMeche changed the title Remove the WorkerGlobalScope from global Use globalThis for global Jun 11, 2023
@JeanMeche
Copy link
Member Author

JeanMeche commented Jun 11, 2023

@dylhunn Following @alfaproject's comment, I change the scope of the PR to actually generalize the behaviour !

@JeanMeche JeanMeche force-pushed the worker-scope branch 5 times, most recently from 9582071 to e7ce29a Compare June 11, 2023 18:05
alfaproject
alfaproject reviewed Jun 11, 2023
View reviewed changes
devversion
devversion approved these changes Jun 19, 2023
View reviewed changes
Copy link
Member

@devversion devversion left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems reasonable to me. I think the current logic is mostly a historic artifact

@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Jun 19, 2023

@alan-agius4 do you see any risks from SSR standpoint?

alan-agius4
alan-agius4 approved these changes Jun 20, 2023
View reviewed changes
Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

JeanMeche added 4 commits June 27, 2023 23:30
@JeanMeche
refactor(core): use globalThis for global
7dbe20e 
`globalThis` is now available on every runtime supported by Angular
@JeanMeche
refactor(localize): use globalThis for global
b8d79ad 
`globalThis` is now available on every runtime supported by Angular
@JeanMeche
refactor(compiler): use globalThis for global
2549894 
`globalThis` is now available on every runtime supported by Angular
@JeanMeche
refactor(platform-browser): use globalThis for global
9afa26d 
`globalThis` is now available on every runtime supported by Angular
jessicajaniuk pushed a commit that referenced this pull request Jul 14, 2023
@JeanMeche @thePunderWoman
refactor(localize): use globalThis for global (#50063)
8aec8d3 
`globalThis` is now available on every runtime supported by Angular

PR Close #50063
jessicajaniuk pushed a commit that referenced this pull request Jul 14, 2023
@JeanMeche @thePunderWoman
refactor(compiler): use globalThis for global (#50063)
7b23d4d 
`globalThis` is now available on every runtime supported by Angular

PR Close #50063
jessicajaniuk pushed a commit that referenced this pull request Jul 14, 2023
@JeanMeche @thePunderWoman
refactor(platform-browser): use globalThis for global (#50063)
67df5a9 
`globalThis` is now available on every runtime supported by Angular

PR Close #50063
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@JeanMeche @sunilbaba
refactor(core): use globalThis for global (angular#50063)
2d05455 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@JeanMeche @sunilbaba
refactor(localize): use globalThis for global (angular#50063)
80862c5 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@JeanMeche @sunilbaba
refactor(compiler): use globalThis for global (angular#50063)
5356705 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
@JeanMeche @sunilbaba
refactor(platform-browser): use globalThis for global (angular#50063)
16c7901 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
@JeanMeche JeanMeche deleted the worker-scope branch August 3, 2023 13:37
@JoostK JoostK mentioned this pull request Aug 16, 2023
Closed
@JeanMeche JeanMeche mentioned this pull request Aug 16, 2023
Closed
JeanMeche added a commit to JeanMeche/angular that referenced this pull request Aug 16, 2023
@JeanMeche
fix(core): guard the jasmine hooks
3198a86 
This commit fixes a regression introduced in angular#50063

Fixes angular#51382
AndrewKushnir pushed a commit that referenced this pull request Aug 21, 2023
@JeanMeche @AndrewKushnir
fix(core): guard the jasmine hooks (#51394)
a9b3c00 
This commit fixes a regression introduced in #50063

Fixes #51382

PR Close #51394
AndrewKushnir pushed a commit that referenced this pull request Aug 21, 2023
@JeanMeche @AndrewKushnir
fix(core): guard the jasmine hooks (#51394)
36f434e 
This commit fixes a regression introduced in #50063

Fixes #51382

PR Close #51394
@Yogu Yogu mentioned this pull request Aug 24, 2023
Closed
14 tasks
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 3, 2023

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 Sep 3, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(core): use globalThis for global (angular#50063)
4d0ebdc 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(localize): use globalThis for global (angular#50063)
965f068 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(compiler): use globalThis for global (angular#50063)
a6267fa 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(platform-browser): use globalThis for global (angular#50063)
6d82a7a 
`globalThis` is now available on every runtime supported by Angular

PR Close angular#50063
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@devversion devversion devversion approved these changes

@alan-agius4 alan-agius4 alan-agius4 approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

+1 more reviewer

@alfaproject alfaproject alfaproject left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

8 participants

@JeanMeche @dylhunn @alfaproject @AndrewKushnir @jessicajaniuk @devversion @alan-agius4 @pkozlowski-opensource