FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: stop installing fetch if `no_browser_globals` is `true` by RaisinTen · Pull Request #41969 · nodejs/node · GitHub

/ node Public

lib: stop installing fetch if no_browser_globals is true - #41969

Merged
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
RaisinTen:lib/stop-installing-fetch-if-no_browser_globals-is-true
Feb 16, 2022
ConversationCommits (1)ChecksFiles changed
Merged

lib: stop installing fetch if no_browser_globals is true#41969
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
RaisinTen:lib/stop-installing-fetch-if-no_browser_globals-is-true

Conversation

RaisinTen commented Feb 14, 2022

Copy link
Copy Markdown
Member

Fixes: #41816
Signed-off-by: Darshan Sen raisinten@gmail.com

lib: stop installing fetch if no_browser_globals is true
6f7bae1 
Fixes: nodejs#41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

nodejs-github-bot commented Feb 14, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 14, 2022
RaisinTen mentioned this pull request Feb 14, 2022
Closed

aduh95 commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

I think if a user use --experimental-fetch, they do want the global fetch. If #41811 lands, then it would make sense to make the default value of --experimental-fetch equal to no_browser_globals, but until then I don't think we should restrict it more.

targos commented Feb 14, 2022

Copy link
Copy Markdown
Member

@aduh95 I think no_browser_globals is here because there were issues with Electron and globals that exist both in Node.js and Chromium. If a user has --experimental-fetch in their NODE_OPTIONS, it could still be a problem.

aduh95 commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

If we're doing this, the same check should be added for

node/lib/internal/bootstrap/pre_execution.js

Lines 169 to 172 in 849991c

function setupWebCrypto() {
if (!getOptionValue('--experimental-global-webcrypto')) {
return;
}

Mesteery added a commit to Mesteery/node that referenced this pull request Feb 14, 2022
lib: stop installing webcrypto if no_browser_globals is true
15bcb4c 
Refs: nodejs#41969 (comment)
Mesteery mentioned this pull request Feb 14, 2022
Merged
panva added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022

This comment was marked as outdated.

Sign in to view
github-actions Bot mentioned this pull request Feb 15, 2022
Open
20 tasks

nodejs-github-bot commented Feb 15, 2022

Copy link
Copy Markdown
Collaborator

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

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 15, 2022
RaisinTen mentioned this pull request Feb 15, 2022
Closed
github-actions Bot mentioned this pull request Feb 16, 2022
Open
33 tasks
Mesteery added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2022
nodejs-github-bot merged commit a82c1a1 into nodejs:master Feb 16, 2022

nodejs-github-bot commented Feb 16, 2022

Copy link
Copy Markdown
Collaborator

Landed in a82c1a1

github-actions Bot mentioned this pull request Feb 17, 2022
Open
28 tasks
RaisinTen deleted the lib/stop-installing-fetch-if-no_browser_globals-is-true branch February 17, 2022 04:49
nodejs-github-bot pushed a commit that referenced this pull request Feb 17, 2022
lib: stop installing webcrypto if no_browser_globals is true
fdab137 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
github-actions Bot mentioned this pull request Feb 18, 2022
Open
26 tasks
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
lib: stop installing fetch if no_browser_globals is true
bd4e550 
Fixes: nodejs#41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#41969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
lib: stop installing webcrypto if no_browser_globals is true
2ab877f 
Refs: nodejs#41969 (comment)

PR-URL: nodejs#41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
lib: stop installing fetch if no_browser_globals is true
1f1198d 
Fixes: nodejs#41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#41969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bengl pushed a commit that referenced this pull request Feb 21, 2022
lib: stop installing fetch if no_browser_globals is true
6d8fb59 
Fixes: #41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #41969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bengl pushed a commit that referenced this pull request Feb 21, 2022
lib: stop installing webcrypto if no_browser_globals is true
9c783c3 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
bengl mentioned this pull request Feb 21, 2022
Merged
bengl pushed a commit that referenced this pull request Feb 21, 2022
lib: stop installing fetch if no_browser_globals is true
8c912ab 
Fixes: #41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #41969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bengl pushed a commit that referenced this pull request Feb 21, 2022
lib: stop installing webcrypto if no_browser_globals is true
53e2e8c 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
bengl pushed a commit that referenced this pull request Feb 22, 2022
lib: stop installing fetch if no_browser_globals is true
68ec8d5 
Fixes: #41816
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #41969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bengl pushed a commit that referenced this pull request Feb 22, 2022
lib: stop installing webcrypto if no_browser_globals is true
c3b0e72 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>

This comment was marked as resolved.

Sign in to view

aduh95 commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

no_browser_globals is not available on v16.x because #40532 never landed there.

danielleadams pushed a commit that referenced this pull request Apr 21, 2022
lib: stop installing webcrypto if no_browser_globals is true
6898161 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit that referenced this pull request Apr 24, 2022
lib: stop installing webcrypto if no_browser_globals is true
e80043f 
Refs: #41969 (comment)

PR-URL: #41971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

panva panva approved these changes
benjamingr benjamingr approved these changes
lpinca lpinca approved these changes
+3 more reviewers
Linkgoron Linkgoron approved these changes
targos targos approved these changes
Mesteery Mesteery approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fetch should not be installed if no_browser_globals==true

10 participants

Back | FazBrowse Home | New Git URL