-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
Closed
Labels
buildIssues and PRs related to build files or the CI.Issues and PRs related to build files or the CI.
Description
Problems
- We have a lot of replaceable codes with primordials
- Accessing built-in objects is restricted by existing rule (no-restricted-globals), but accessing a property in the built-in objects is not restricted.
- We review codes that can be replaced by primordials, but there's a lot of codes that need to be fixed.
- Here is the list of replaceable codes on the commit 4d7015f
- We have often made a pull request to replace codes that use a global built-in object with primordials
- For example: lib: replace every Symbol.for by SymbolFor #30857 and lib: replace Array to ArrayIsArray by primordials #32258 (see search results)
- Can we fix codes before merging them?
Proposal
I created a new draft PR that includes a new custom ESLint rule for that.
#35448
- It'll report accessing global built-in objects that aren't using primordials such
ArrayorSymbol - It'll report static methods such as
Array.fromorSymbol.for - It won't report prototype methods to prevent false-positive reporting. It will need static type information.
Here is the result of make lint-js with new rule on the commit 4d7015f.
If we have a special reason for using the built-in object, we can add an inline comment such as eslint-disable node-core/prefer-primordials.
Pros
- Improve security
- Improve performance
- Reduce reviewing cost
- No need PRs that replace codes with primordials anymore.
Concerns
I'm not sure whether all codes in lib/ should be replaced with primordials.
What do you think?
watilde
Metadata
Metadata
Assignees
Labels
buildIssues and PRs related to build files or the CI.Issues and PRs related to build files or the CI.