Prevent extension when calling makeImmutable(), and add pinning tests by jdmarshall · Pull Request #866 · node-config/node-config

jdmarshall · 2026-01-20T01:00:31Z

Ask JavaScript if the object is mutable instead of trying to track state

Fix for both bugs in #865, and makes #864 more agreeable to honoring the immutability contract.

Note while this PR is marked as a breaking change, absolutely none of our existing tests failed with this change,
meaning we have documented behavior that we do not test for.

Looking into the commit history, the contrary comment in the JSDoc dates to a time when this function was about
eight lines long and had none of the other many, many additions to enforce immutability. I propose that this is not actually a useful feature anymore. But I will mark this as 4.4 which will have several other breaking changes, to leave 4.3 more or less in peace.

Summary by CodeRabbit

Refactor
- Updated immutability behavior to use a runtime extensibility check and added partial-mode so selected top-level properties can remain extensible while nested settings stay immutable.
Tests
- Added tests ensuring immutability holds after sequential accesses and that adding new properties to an immutable configuration is prevented.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-20T01:00:42Z

📝 Walkthrough

Walkthrough

Replaces the first-get mutability flag with a runtime Object.isExtensible(config) check; refactors util.makeImmutable to add partial/full modes and consolidate Object.preventExtensions usage; removes explicit mutability-reset logic in setModuleDefaults; adds tests asserting top-level and nested immutability.

Changes

Cohort / File(s)	Summary
Core immutability & API `lib/config.js`	Remove per-first-get mutability flag; use `Object.isExtensible(config)` to decide immutabilization; eliminate mutability reset in `setModuleDefaults`; keep exported signatures unchanged.
makeImmutable behavior `lib/...util.makeImmutable` (within `lib/config.js`)	Add partial mode when passed an array of property names; in full mode call `Object.preventExtensions(object)` once on the root; in partial mode allow root extensibility while freezing/wrapping specified properties and preserving proxy-based nested immutability.
Tests `test/2-config-test.js`	Add tests: confirm mutation prevented after nested `get()` sequence; assert adding new top-level fields to a fully immutable module throws non-extensible error.

Sequence Diagram(s)

(Skipped — changes are internal control-flow and immutability behavior, not multi-component interaction flows.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

makeImmutable() only makes the children immutable, not the target object #865 — The PR's changes to makeImmutable (ensuring top-level preventExtensions and adding partial/full semantics) align with the issue describing root object remaining extensible while children were immutable.

Suggested reviewers

markstos

Poem

🐰 I nibble keys and smooth each seam,
I hop through configs, calm and keen.
No flapping flags — the root stands tight,
New fields bounce off, surprised by the bite.
Hooray for stable config dreams! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: preventing extension when calling makeImmutable() and adding pinning tests, which aligns with the core modifications in both config.js and test files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

jdmarshall · 2026-01-20T04:00:20Z

This change may have some performance implications. Running some benchmarks to check.

Follow-up: Only inside Webstorm. Not reproducible from the command line.

jdmarshall · 2026-01-20T04:06:13Z

@coderabbitai review

coderabbitai · 2026-01-20T04:06:19Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@lib/config.js`:
- Around line 352-356: Update the JSDoc to reflect that when makeImmutable is
called with no property list (i.e., partial is falsy) the target object is made
non‑extensible via Object.preventExtensions(object), so new properties cannot be
added; locate the documentation near the makeImmutable/partial logic and change
the comment that currently says new properties can be added to state the
opposite: in full mode the object is non‑extensible and new properties are
prohibited (mention Object.preventExtensions and the partial flag).

lib/config.js

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/config.js (1)

207-223: setModuleDefaults with new modules after get() will fail with immutability enabled.

Calling setModuleDefaults with a new module name after the first get() call (when ALLOW_CONFIG_MUTATIONS is false by default) will throw an error. The Util.setPath call at line 216 attempts to create intermediate objects by assigning to a proxy-wrapped immutable config, triggering the proxy's set trap which rejects adding new properties. This is a breaking change from prior behavior where mutability could be reset.

♻️ Duplicate comments (1)

lib/config.js (1)

231-232: JSDoc partially updated but still incomplete.

The doc update here is accurate. However, as previously noted, the JSDoc further down (around lines 233-235 per the past review) still states that new properties can be added, which contradicts the new Object.preventExtensions behavior in full mode. Please address the outstanding comment to complete the documentation alignment.

🧹 Nitpick comments (1)

lib/config.js (1)
269-276: Logic for partial mode is correct.

The partial flag cleanly differentiates between freezing specific properties (array provided) versus full immutability (no array). This preserves backward compatibility while enabling the new non-extensible behavior for the default case.

Minor style nit: Line 270 has the comment attached without a space after null;.
🔧 Optional formatting fix
-  let properties = null;// Get the list of properties to work with
+  let properties = null; // Get the list of properties to work with

…rack state. Fix for both bugs in node-config#865

jdmarshall · 2026-01-20T04:33:21Z

setModuleDefaults with new modules after get() will fail with immutability enabled.

That's documented as unsupported. Where do you see the opposite mentioned in the docs?

the JSDoc further down (around lines 233-235 per the past review) still states that new properties can be added, which contradicts the new Object.preventExtensions behavior in full mode.

Now you're hallucinating. 233-235 has always been an empty paragraph. Unless there's some whitespace morse code I'm not seeing...

jdmarshall · 2026-02-07T05:47:40Z

I'm going to split this in two.

jdmarshall force-pushed the immutabilyBuddy branch from 2d35760 to e14f228 Compare January 20, 2026 01:06

jdmarshall changed the title ~~Prevent extension when calling makeImmutable(), and missed checks on parent objects~~ Prevent extension when calling makeImmutable(), and add pinning tests Jan 20, 2026

jdmarshall force-pushed the immutabilyBuddy branch from e14f228 to d3719cf Compare January 20, 2026 01:42

jdmarshall mentioned this pull request Jan 20, 2026

withModuleDefaults() #864

Open

coderabbitai bot reviewed Jan 20, 2026

View reviewed changes

lib/config.js Show resolved Hide resolved

jdmarshall force-pushed the immutabilyBuddy branch from d3719cf to bd1103e Compare January 20, 2026 04:19

jdmarshall added the breaking-change label Jan 20, 2026

jdmarshall added this to the 4.4 milestone Jan 20, 2026

coderabbitai bot reviewed Jan 20, 2026

View reviewed changes

Ask JavaScript if the config object is mutable instead of trying to t…

6e6fff9

…rack state. Fix for both bugs in node-config#865

jdmarshall force-pushed the immutabilyBuddy branch from bd1103e to 6e6fff9 Compare January 20, 2026 04:29

jdmarshall closed this Feb 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent extension when calling makeImmutable(), and add pinning tests#866

Prevent extension when calling makeImmutable(), and add pinning tests#866
jdmarshall wants to merge 1 commit intonode-config:masterfrom
jdmarshall:immutabilyBuddy

jdmarshall commented Jan 20, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 20, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Suggested reviewers

Poem

Uh oh!

jdmarshall commented Jan 20, 2026 •

edited

Loading

Uh oh!

jdmarshall commented Jan 20, 2026

Uh oh!

coderabbitai bot commented Jan 20, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

jdmarshall commented Jan 20, 2026 •

edited

Loading

Uh oh!

jdmarshall commented Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jdmarshall commented Jan 20, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Suggested reviewers

Poem

Uh oh!

jdmarshall commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdmarshall commented Jan 20, 2026

Uh oh!

coderabbitai bot commented Jan 20, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jdmarshall commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdmarshall commented Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdmarshall commented Jan 20, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 20, 2026 •

edited

Loading

jdmarshall commented Jan 20, 2026 •

edited

Loading

jdmarshall commented Jan 20, 2026 •

edited

Loading