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.

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

lib: enforce using primordials.globalThis instead of global #38230

Merged
merged 1 commit into from
Apr 17, 2021
Merged

lib: enforce using primordials.globalThis instead of global #38230

merged 1 commit into from
Apr 17, 2021

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 13, 2021 •
edited
Loading

global.global may be overwritten in user-land, it's probably safer not to rely on it.

$ echo 'Reflect.defineProperty(globalThis, "global", { get() {throw new Error} })' > pre.js
$ node -r ./pre.js
Welcome to Node.js v15.12.0.
Type ".help" for more information.
/Users/duhamean/Documents/node/f.js:2
  get() {throw new Error}
               ^

Error
    at get (…/pre.js:1:61)
    at REPLServer.createContext (node:repl:1011:5)
    at REPLServer.resetContext (node:repl:1059:23)
    at new REPLServer (node:repl:720:8)
    at Object.start (node:repl:982:10)
    at Object.createRepl [as createInternalRepl] (node:internal/repl:51:21)
    at node:internal/main/repl:41:13
    at Object.loadESM (node:internal/process/esm_loader:68:11)

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 13, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/37444/ 💚

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 14, 2021
@jasnell jasnell removed the needs-ci PRs that need a full CI run. label Apr 14, 2021
@aduh95
lib: enforce using primordials.globalThis instead of global
d756d2b 
PR-URL: nodejs#38230
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@aduh95
Copy link
Contributor Author

aduh95 commented Apr 17, 2021

Landed in d756d2b

@aduh95 aduh95 merged commit d756d2b into nodejs:master Apr 17, 2021
@aduh95 aduh95 deleted the global-primordials branch April 17, 2021 13:31
@BethGriggs BethGriggs mentioned this pull request Apr 20, 2021
Merged
aduh95 added a commit to aduh95/node that referenced this pull request Jul 19, 2021
@aduh95
lib: enforce using primordials.globalThis instead of global
3eb7008 
PR-URL: nodejs#38230
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@aduh95 aduh95 mentioned this pull request Jul 19, 2021
Closed
richardlau pushed a commit that referenced this pull request Jul 20, 2021
@aduh95 @richardlau
lib: enforce using primordials.globalThis instead of global
2ead4d2 
PR-URL: #38230
Backport-PR-URL: #39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@richardlau
Copy link
Member

Backported to v14.x-staging in #39448.

richardlau pushed a commit that referenced this pull request Jul 20, 2021
@aduh95 @richardlau
lib: enforce using primordials.globalThis instead of global
d4f96bb 
PR-URL: #38230
Backport-PR-URL: #39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@richardlau richardlau mentioned this pull request Jul 20, 2021
Merged
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@aduh95 @foxxyz
lib: enforce using primordials.globalThis instead of global
53803f2 
PR-URL: nodejs#38230
Backport-PR-URL: nodejs#39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
aduh95 added a commit to aduh95/node that referenced this pull request Feb 16, 2022
@aduh95
tools,lib: remove global and Intl from the list of globals
4d3de28 
Refs: nodejs#38230
Refs: nodejs#38419
@aduh95 aduh95 mentioned this pull request Feb 16, 2022
Merged
nodejs-github-bot pushed a commit that referenced this pull request Feb 18, 2022
@aduh95
tools,lib: remove global and Intl from the list of globals
3aab986 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
53bbbc8 
Refs: nodejs#38230
Refs: nodejs#38419

PR-URL: nodejs#42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
749b4fd 
Refs: nodejs#38230
Refs: nodejs#38419

PR-URL: nodejs#42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
1ea4726 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
a87fa5b 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 22, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
cfafb45 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this pull request Apr 21, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
1dfbf3e 
Refs: nodejs#38230
Refs: nodejs#38419

PR-URL: nodejs#42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 24, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
80f944f 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 24, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
4bba279 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@aduh95 @nodejs-github-bot @richardlau @jasnell @Trott @addaleax @lpinca @RaisinTen @VoltrexKeyva @targos