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

lib: enhance use of Map with primordials by Sebastien-Ahkrin · Pull Request #31155 · nodejs/node · GitHub

/ node Public

lib: enhance use of Map with primordials - #31155

Closed
Sebastien-Ahkrin wants to merge 1 commit into
nodejs:masterfrom
Sebastien-Ahkrin:MapPrimordials
ConversationCommits (1)ChecksFiles changed
Closed

lib: enhance use of Map with primordials#31155
Sebastien-Ahkrin wants to merge 1 commit into
nodejs:masterfrom
Sebastien-Ahkrin:MapPrimordials

Conversation

Sebastien-Ahkrin commented Jan 2, 2020

Copy link
Copy Markdown
Contributor

Hello,
Now, in this PR I have added Map in the primordials eslint
so have created a line in "/lib/.eslintrc.yaml".

rules:
  no-restricted-globals:
  - name: Map
        message: "Use `const { Map } = primordials;` instead of the global."

And just import Map in some file :).

const {
  // [...]
  Map,
} = primordials;

I hope this new PR will help you :x

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jan 2, 2020

Sebastien-Ahkrin commented Jan 2, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Refs: #30697

BridgeAR commented Jan 2, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I know we also have a SafeMap. Do we still need both? Or is it enough to rely upon the primordial Map? Similar would apply to Set and their weak counterparts.

// cc @devsnek @targos

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 2, 2020

nodejs-github-bot commented Jan 2, 2020

Copy link
Copy Markdown
Collaborator

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

targos commented Jan 2, 2020

Copy link
Copy Markdown
Member

SafeMap is not exactly the same, as it also provides safe calls to the prototype methods.

nodejs-github-bot commented Jan 3, 2020

Copy link
Copy Markdown
Collaborator

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

Trott commented Jan 4, 2020

Copy link
Copy Markdown
Member

Commit message might be clearer as lib: use primordial for Map? That can be edited by whoever lands this, though.

nodejs-github-bot commented Jan 4, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 4, 2020

Copy link
Copy Markdown
Collaborator

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

Trott commented Jan 4, 2020

Copy link
Copy Markdown
Member

Landed in 9085c03

Trott closed this Jan 4, 2020
Trott pushed a commit that referenced this pull request Jan 4, 2020
lib: replace Map global by the primordials
9085c03 
PR-URL: #31155
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Jan 6, 2020
lib: replace Map global by the primordials
20ecb5d 
PR-URL: #31155
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BridgeAR mentioned this pull request Jan 7, 2020
Merged
targos pushed a commit that referenced this pull request Jan 14, 2020
lib: replace Map global by the primordials
0dd8605 
PR-URL: #31155
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
lib: replace Map global by the primordials
947d066 
PR-URL: #31155
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins mentioned this pull request Feb 8, 2020
Merged
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

jasnell jasnell approved these changes
Trott Trott approved these changes
cjihrig cjihrig approved these changes
BridgeAR BridgeAR approved these changes
trivikr trivikr approved these changes
+2 more reviewers
targos targos approved these changes
devnexen devnexen 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. 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.

9 participants

Back | FazBrowse Home | New Git URL