Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Nov 23, 2025

Closes: #60795

I'm not sure if we should treat this as a breaking change.
Before this change, indexed properties would not be intercepted, so setting them in JS would not coerce the value to a string.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Nov 23, 2025
@targos targos added the process Issues and PRs related to the process subsystem. label Nov 23, 2025
@targos targos force-pushed the process-env-indexed branch from 5a0fbfe to 9cda07b Compare November 23, 2025 14:50
@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

❌ Patch coverage is 70.37037% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.56%. Comparing base (430db0d) to head (ef1b42c).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/node_env_var.cc 70.37% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60826      +/-   ##
==========================================
+ Coverage   88.55%   88.56%   +0.01%     
==========================================
  Files         703      703              
  Lines      208237   208264      +27     
  Branches    40157    40162       +5     
==========================================
+ Hits       184395   184448      +53     
+ Misses      15858    15827      -31     
- Partials     7984     7989       +5     
Files with missing lines Coverage Δ
src/node_env_var.cc 82.21% <70.37%> (-0.93%) ⬇️

... 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.

@Renegade334
Copy link
Member

Are there consequences from having the named property enumerator emit numeric values? (I know that's what happens currently...)

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 23, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2025
@nodejs-github-bot
Copy link
Collaborator

@targos targos marked this pull request as draft November 25, 2025 07:00
@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 25, 2025
@targos targos force-pushed the process-env-indexed branch from 9cda07b to 57da1d7 Compare November 27, 2025 07:33
@targos targos marked this pull request as ready for review November 27, 2025 07:33
@targos targos force-pushed the process-env-indexed branch from 57da1d7 to ef1b42c Compare November 27, 2025 07:33
@targos targos changed the title src: handled indexed properties in process.env src: handle indexed properties in process.env Nov 27, 2025
@targos
Copy link
Member Author

targos commented Nov 27, 2025

Rebased to include #60846 and fixed a typo in the commit message.

@targos
Copy link
Member Author

targos commented Nov 27, 2025

Are there consequences from having the named property enumerator emit numeric values? (I know that's what happens currently...)

@Renegade334 I think it only affects users of the C++ API, which allows to skip indices.

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2025
@nodejs-github-bot nodejs-github-bot merged commit 08d966c into nodejs:main Nov 27, 2025
66 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 08d966c

@targos targos deleted the process-env-indexed branch November 27, 2025 13:17
targos added a commit that referenced this pull request Nov 29, 2025
Closes: #60795
PR-URL: #60826
Fixes: #60795
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

process.env is missing environment variables with numeric names

8 participants