Skip to content

Conversation

himself65
Copy link
Member

No description provided.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Jul 17, 2025
@jasnell
Copy link
Member

jasnell commented Jul 17, 2025

Can I ask you to include a brief description of what issue this PR is addressing or what it is changing in the PR text? :-)

return ReflectSet(target, prop, value);
},
},
);
Copy link
Member

@jasnell jasnell Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of performance impact will this have? inspect.colors is accessed in a for loop inside styleText and accessing via a Proxy (even without a get trap) will have a definite performance impact in such cases, so we should be careful here.

@himself65
Copy link
Member Author

himself65 commented Jul 17, 2025

I was thinking to make sure inspect.colors is checked type in runtime (from PR #59098). But just realized this is not gonna work. And yeah it's cause performance issue. So im closing this PR

@himself65 himself65 closed this Jul 17, 2025
Copy link

codecov bot commented Jul 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.04%. Comparing base (48fff6b) to head (07ecf95).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59107      +/-   ##
==========================================
- Coverage   90.05%   90.04%   -0.01%     
==========================================
  Files         645      645              
  Lines      189153   189171      +18     
  Branches    37093    37099       +6     
==========================================
+ Hits       170339   170342       +3     
- Misses      11518    11528      +10     
- Partials     7296     7301       +5     
Files with missing lines Coverage Δ
lib/internal/util/inspect.js 99.96% <100.00%> (+0.03%) ⬆️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants