-
Notifications
You must be signed in to change notification settings - Fork 16
fix(globals): use globals
for globals
#397
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.
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the hardcoded list of JavaScript global objects to use the globals
library, which automatically updates with new ES specifications monthly. This change simplifies maintenance by removing the need to manually track and update global objects.
- Replaces hardcoded arrays of JavaScript globals with dynamic imports from the
globals
library - Moves the
globals
dependency from devDependencies to dependencies - Simplifies the primitive types mapping from an object to an array structure
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/utils/parser/constants.mjs |
Replaces manual global object lists with globals.builtin and converts primitive mapping to array |
src/utils/parser/index.mjs |
Updates primitive type transformation to work with new array-based mapping |
package.json |
Moves globals from devDependencies to dependencies |
.github/dependabot.yml |
Removes globals from linting dependency group |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #397 +/- ##
==========================================
- Coverage 74.29% 74.20% -0.09%
==========================================
Files 118 118
Lines 11025 10987 -38
Branches 695 695
==========================================
- Hits 8191 8153 -38
Misses 2831 2831
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
globals\
for globalsglobals
for globals
Blocked by nodejs/node#59421 |
Unrelated, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going down this approach, wouldn't it be easier to use Object.keys(self)
?
cc @aduh95 / @nodejs/collaborators |
No, firstly, see #397 (comment). Secondly, we aren't building the docs on for same version of Node.js as we are on, so the list would be incomplete |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as off-topic.
This comment was marked as off-topic.
Need to fix one thing, then this'll be good to go |
Description
The
globals
library is updated with every builtin global every month (https://github.com/sindresorhus/globals/blob/main/.github/workflows/update.yml), so we can safely rely on it to populate our builtin globals.IMO this is a safer alternative than having us manually update this list.
Related Issues
Ref #319