Rework raw.js to function like the new defer mechanism. by jdmarshall · Pull Request #879 · node-config/node-config

jdmarshall · 2026-02-13T01:03:06Z

This should be the last part of the config API that blocks ESM usage.

addresses #878

Summary by CodeRabbit

New Features
- Added a RawConfig utility to mark/preserve raw objects through config processing.
- Executable config bootstrap callbacks now receive raw access in their context.
Deprecations
- Legacy raw() helper deprecated — prefer the RawConfig utility or passing raw via config callbacks.

coderabbitai · 2026-02-13T01:03:23Z

📝 Walkthrough

Walkthrough

Adds a public RawConfig class in lib/util.js, exposes RawConfig.raw via raw.js (with deprecation delegation), and threads RawConfig.raw into the bootstrap callback context for function-based config files; updates imports in lib/config.js to source RawConfig from lib/util.js.

Changes

Cohort / File(s)	Summary
Core RawConfig & util changes `lib/util.js`	Adds `RawConfig` class and `RawConfig.raw()` factory; treats `RawConfig` instances as pass-through in `cloneDeep`, `equalsDeep`, and deep-extend logic; includes `raw` in bootstrap callback context; exports `RawConfig`.
raw delegation & deprecation `raw.js`	Replaces wrapper implementation with `raw(rawObj)` that delegates to `RawConfig.raw(rawObj)` and emits a deprecation notice via `Util.errorOnce()`; preserves public exports.
Config import update `lib/config.js`	Switches to import `RawConfig` from `./util.js` alongside `Util` and `Load` (replaces previous import from `../raw`); no behavior change beyond import source.

Sequence Diagram(s)

sequenceDiagram
  participant Loader as Load
  participant Util as Util
  participant Raw as RawConfig
  participant ConfigFn as Config File (function)
  Loader->>ConfigFn: invoke({ defer, util: Util, raw: Raw.raw })
  ConfigFn->>Raw: (optional) call Raw.raw(...)
  ConfigFn-->>Loader: return config object
  Loader->>Util: merge/clone config (RawConfig treated as pass-through)
  Loader-->>Caller: final config

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

markstos

Poem

🐇 I hopped through code to hide a clue,
Wrapped raw bits so they stayed true,
Bootstrap now carries the carrot I bring,
Quiet and raw, a precious thing,
Hooray — config hops, ready to spring!

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 change: refactoring raw.js to function like the defer mechanism, which is the core purpose evident from the code changes.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into `master`

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

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

lib/util.js (3)
23-25: JSDoc @param for context is missing the type for raw.

The raw property in the context object lacks a type annotation. Consider:
- * `@param` context { util: Util, defer: deferConfig, raw } - utility functions for startup
+ * `@param` context {{ util: Util, defer: deferConfig, raw: function }} - utility functions for startup
453-454: RawConfig should return child early, consistent with the Buffer branch above.

Setting child = parent without returning causes the instance to fall through into the circular-reference tracking and the for…in property loop. It works today only because the private field isn't enumerable, but an early return is clearer, consistent with the Buffer pattern, and future-proof.
Proposed fix
       } else if (parent instanceof RawConfig) {
-        child = parent;
+        return parent;
       } else {
640-659: Pre-existing: Date branch at line 640 falls through into subsequent conditions.

The if (mergeFrom[prop] instanceof Date) block assigns mergeInto[prop] but lacks an else before the RegExp/RawConfig check, so execution continues into the chain below. Your new RawConfig addition correctly mirrors the RegExp pattern, but the overall structure means a Date value will be assigned and then re-evaluated by the else if chain. Not introduced by this PR, just flagging for awareness.
raw.js (1)
1-12: Clean deprecation delegation — looks good.

The errorOnce approach ensures the warning fires at most once, and the delegation to RawConfig.raw is straightforward.

One minor inconsistency: the require path './lib/util' omits the .js extension, while other internal requires in the project (e.g., require('./util.js') in lib/config.js) include it. Consider adding .js for consistency and ESM-readiness, especially since this PR is specifically about unblocking ESM usage.
Optional fix
-const { Util, RawConfig } = require('./lib/util')
+const { Util, RawConfig } = require('./lib/util.js')

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

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@lib/util.js`:
- Around line 1020-1023: Fix the JSDoc typo: change the annotation `@type
defebootstrapCallback` to the correct `@type {bootstrapCallback}` so the
documented type matches the actual callback definition; update the comment
immediately above where `let fn = configObject;` is declared (referencing
symbols `configObject`, `fn`) and ensure the typedef name `bootstrapCallback`
matches the callback used earlier in the file (lines that reference `Util` and
`RawConfig.raw`).

In `@raw.js`:
- Around line 1-6: lib/config.js currently imports RawConfig indirectly via
raw.js (which calls Util.errorOnce and delegates to RawConfig.raw), creating a
dependency on a deprecated module; change the require in lib/config.js to import
RawConfig directly from lib/util (the module that defines and exports RawConfig)
and update any references to RawConfig there to use the directly imported symbol
so behavior is unchanged but the deprecated internal dependency is removed.

🧹 Nitpick comments (2)

lib/util.js (2)
453-454: Consider an early return for RawConfig in cloneDeep, similar to the Buffer branch.

Currently child = parent falls through to the circular-reference tracking and the for…in property loop. Since RawConfig has no enumerable own properties, the loop is a no-op, but an early return would make the intent clearer and avoid unnecessary work.
Proposed fix
       } else if (parent instanceof RawConfig) {
-        child = parent;
+        return parent;
       } else {
23-24: raw property is missing its type in the @param JSDoc.

The context object documents types for util and defer but omits the type for raw. For consistency and IDE support, consider adding it.
Proposed fix
- * `@param` context { util: Util, defer: deferConfig, raw } - utility functions for startup
+ * `@param` context {{ util: Util, defer: deferConfig, raw: RawConfig.raw }} - utility functions for startup

lib/util.js

raw.js

… to 4.3

Deprecatng the old mechanism, as it blocks ESM migration. addresses node-config#878

coderabbitai

Caution

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

⚠️ Outside diff range comments (1)

lib/util.js (1)
640-659: ⚠️ Potential issue | 🔴 Critical

Breaking the if/else chain causes double-processing of Date (and other) values.

The if on line 644 is no longer chained as an else if to the Date check on line 640. This means when mergeFrom[prop] is a Date:

Line 641 assigns it correctly.

Execution falls through to line 644's if, which doesn't match.

The else if on line 647 sees both sides as objects (typeof Date === 'object') and calls extendDeep on the Date, corrupting it.

Re-attach the chain with else if:
🐛 Proposed fix
       if (mergeFrom[prop] instanceof Date) {
         mergeInto[prop] = mergeFrom[prop];
-      }
-
-      if ((mergeFrom[prop] instanceof RegExp) ||
+      } else if ((mergeFrom[prop] instanceof RegExp) ||
           (mergeFrom[prop] instanceof RawConfig)) {

🧹 Nitpick comments (1)

lib/util.js (1)

16-26: Minor JSDoc formatting for @callback parameter.

The @param on line 24 uses informal syntax. Consider proper JSDoc:

- * `@param` context { util: Util, defer: deferConfig, raw } - utility functions for startup
+ * `@param` {Object} context - utility functions for startup
+ * `@param` {typeof Util} context.util - Util class
+ * `@param` {typeof deferConfig} context.defer - deferred config factory
+ * `@param` {typeof RawConfig.raw} context.raw - raw config factory

jdmarshall added this to the 4.4 milestone Feb 13, 2026

jdmarshall added the feature-request label Feb 13, 2026

coderabbitai bot reviewed Feb 13, 2026

View reviewed changes

lib/util.js Outdated Show resolved Hide resolved

raw.js Show resolved Hide resolved

jdmarshall force-pushed the rawESM branch from eb8f458 to dd3c1d3 Compare February 13, 2026 06:01

jdmarshall added 2 commits February 12, 2026 22:04

Fix some type errors, add a signature for the callback function added…

f087e50

… to 4.3

Allow raw to function like defer()

f1889d0

Deprecatng the old mechanism, as it blocks ESM migration. addresses node-config#878

jdmarshall force-pushed the rawESM branch from dd3c1d3 to f1889d0 Compare February 13, 2026 06:04

coderabbitai bot reviewed Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework raw.js to function like the new defer mechanism.#879

Rework raw.js to function like the new defer mechanism.#879
jdmarshall wants to merge 2 commits intonode-config:masterfrom
jdmarshall:rawESM

jdmarshall commented Feb 13, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Feb 13, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jdmarshall commented Feb 13, 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 Feb 13, 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

Suggested reviewers

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdmarshall commented Feb 13, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 13, 2026 •

edited

Loading