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

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

Merged
jdmarshall merged 2 commits into
node-config:masterfrom
jdmarshall:rawESM
Feb 17, 2026
Merged

Rework raw.js to function like the new defer mechanism.#879
jdmarshall merged 2 commits into
node-config:masterfrom
jdmarshall:rawESM

Conversation

@jdmarshall

@jdmarshall jdmarshall commented Feb 13, 2026

Copy link
Copy Markdown
Collaborator

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 a raw entry in their context.
  • Deprecations

    • Legacy raw() helper deprecated — now delegates to RawConfig and emits a runtime deprecation warning.
  • Exports

    • RawConfig is exported from the primary utility surface (previous export path consolidated).

@jdmarshall jdmarshall added this to the 4.4 milestone Feb 13, 2026
@coderabbitai

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Comment thread lib/util.js Outdated
Comment thread raw.js
@jdmarshall jdmarshall force-pushed the rawESM branch 2 times, most recently from dd3c1d3 to f1889d0 Compare February 13, 2026 06:04

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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:

  1. Line 641 assigns it correctly.
  2. Execution falls through to line 644's if, which doesn't match.
  3. 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 jdmarshall merged commit 4354ee2 into node-config:master Feb 17, 2026
2 checks passed
@jdmarshall jdmarshall deleted the rawESM branch February 26, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant