Skip to content

Support callbacks in config JS/TS/ files to avoid the need for require() in config files#863

Merged
jdmarshall merged 10 commits intonode-config:masterfrom
jdmarshall:deferredESM
Feb 12, 2026
Merged

Support callbacks in config JS/TS/ files to avoid the need for require() in config files#863
jdmarshall merged 10 commits intonode-config:masterfrom
jdmarshall:deferredESM

Conversation

@jdmarshall
Copy link
Collaborator

@jdmarshall jdmarshall commented Jan 15, 2026

This fixes #740 and provides a solution for a number of issues preventing ESM migration.

At some point Loren added a future compatibility wiki page to the project.

Since wiki edits do not really show up in the project activity, I was not aware of this, and based on recent events, I don't believe @markstos was either. Several of the clauses have been violated in recent months, which some users have already noted.

One of the clauses in this document has no support in the current codebase:

Ample Warning

A minimum of one year will be given between the time of an incompatibility announcement (below) and the release of the version containing the incompatibility.

If it's possible to detect your application using a future incompatibility, a one-time warning per application run will be placed onto console.error for each type of incompatibility.

Once the incompatibility is introduced, an error will be placed into console.error if an incompatible usage is detected.

Summary by CodeRabbit

  • New Features

    • Deferred configuration gains asynchronous (Promise) resolution, lazy evaluation, and utilities to resolve async configs and emit one-time errors.
    • Configuration modules may be exported as factory functions that receive injected helpers.
  • Documentation

    • Deprecation notices and runtime warnings added to guide migration from legacy APIs.
  • Chores

    • Copyright years updated to 2026.
  • Tests

    • Expanded tests for async/deferred resolution; several legacy assertions were removed or simplified.

@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

Replaces the in-file defer implementation with a new lib/defer.js module (exporting deferConfig and DeferredConfig), re-exports deprecated wrappers from defer.js, adds async-capable deferred resolution and Util.resolveAsyncConfigs / Util.errorOnce, invokes config file factories with {defer, util}, updates tests/fixtures, and bumps copyright years.

Changes

Cohort / File(s) Summary
License & Docs
LICENSE, README.md
Bumped copyright year from 2010-2025 to 2010-2026.
Async surface
async.js
Added JSDoc deprecation notes and a runtime one-time deprecation warning for asyncConfig; no public signature changes.
Defer implementation
lib/defer.js
New module providing DeferredConfig constructor and deferConfig(func) supporting lazy sync/async resolution and wiring getters that trigger resolution.
Defer re-export & wrappers
defer.js
Now re-exports from lib/defer.js; provides deprecated wrapper(s) that emit one-time warnings while delegating to new implementations.
Util enhancements
lib/util.js
Adds static async resolveAsyncConfigs(config) and static errorOnce(key, message, ...args); refactors traversal to prepare/resolve DeferredConfig instances (including arrays/standalone values), awaits async leaves, and invokes config factory functions with {defer, util}.
Tests & fixtures
test/0-util.js, test/3-config/default.js, test/3-deferred-configs.js
test/0-util.js: expanded tests for deferred and async resolution; test/3-config/default.js: switched export to factory module.exports = ({defer, util}) => {...}; test/3-deferred-configs.js: removed several legacy tests.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Loader as Config Loader
    participant ConfigFn as Config File (factory)
    participant Util as lib/util
    participant Defer as lib/defer

    App->>Loader: require/load config file
    Loader->>ConfigFn: invoke with ({defer: Defer.deferConfig, util: Util})
    ConfigFn->>Defer: create DeferredConfig via deferConfig(...)
    ConfigFn-->>Loader: return config object (may contain DeferredConfig or Promises)
    Loader->>Util: Util.resolveAsyncConfigs(config)
    Util->>Util: traverse config, call prepare on Deferreds, collect Promises
    Util->>Defer: call resolve on DeferredConfig instances
    Util->>Util: await Promise.all(collectedPromises)
    Util-->>Loader: resolved config object
    Loader-->>App: finalized config returned
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • markstos

Poem

🐰 I hopped through code and moved defer to its den,
I wrapped old calls in warnings and pointed to when,
Util now waits for promises to finish their race,
Factories get {defer, util} — a tidy new place. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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: adding callback support in config files to eliminate require() dependencies, enabling ESM compatibility.
Linked Issues check ✅ Passed The PR implements callback-based deferment (new lib/defer.js, updated async.js with deprecation warnings) and integrates with package loading to support the goal of enabling defer to work with bundlers like esbuild by providing callbacks that don't require module imports in config files.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing the callback deferment feature: new lib/defer.js module, deprecation notices in async.js/defer.js, copyright year updates, utility enhancements, and test updates. No unrelated changes detected.

✏️ 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

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 jdmarshall marked this pull request as draft January 15, 2026 04:44
@jdmarshall jdmarshall marked this pull request as ready for review January 15, 2026 04:53
@jdmarshall
Copy link
Collaborator Author

jdmarshall commented Jan 19, 2026

This feature is critical to breaking the chicken and egg problems with the config singleton and moving this module to ESM. I have a PR almost ready to go for ESM migration that requires this PR to land. I’d appreciate a timely review while I still have momentum.

Because it deprecates several features, I'd like this to be in 4.3 so that people have plenty of time to adapt before I remove those deprecations in 5.0, which will absolutely be necessary in order to get import to work. You cannot import from modules that are currently being imported, and that is exactly what async.js and defer.js are doing.

Using the callback mechanism means we are exposing the parts of node-config that are stateless or have private state, and thus have already finished importing by the time deferment is resolved.

@jdmarshall jdmarshall added this to the 4.3 milestone Jan 19, 2026
@jdmarshall
Copy link
Collaborator Author

I think this is the only route to ESM support and I'd like to land this into 4.3 @markstos @lorenwest

Copy link

@coderabbitai coderabbitai bot left a comment

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 1001-1005: The current factory-path treats a returned Promise as a
config object (configObject = fn({ defer: deferConfig, util: Util })) which
breaks downstream code; change this to call the factory and detect if the result
is a Promise (e.g. result && typeof result.then === 'function') and either await
it (so make the enclosing function async and assign configObject = await result)
or explicitly throw a clear error if async factories are unsupported; update
references to configObject/fn/deferConfig/Util and any callers of the enclosing
function to handle the async behavior accordingly.
- Around line 346-372: _iterate currently skips Promise elements inside arrays
because property.forEach(_iterate) never reaches the instanceof Promise check;
change the array branch in _iterate (where it checks property.constructor ===
Array) to iterate by index instead of forEach, and for each element do: if
element is a Promise, push a promise that assigns the resolved value back into
property[i] (e.g., promises.push(element.then(v => { property[i] = v })));
otherwise call _iterate on the element to recurse; keep the final await
Promise.all(promises) so those pushed resolvers write the resolved values back
into config; refer to _iterate, the array branch, promises array, and the final
Promise.all(promises).
🧹 Nitpick comments (2)
lib/util.js (2)

7-8: Inconsistent file extension in require paths.

Line 7 uses './defer.js' (with extension) while line 8 uses '../async' (without). Consider being consistent.


347-349: Heads-up: constructor-based type checks may break for exotic objects.

prop.constructor === String (and similar checks on Object/Array) will fail for objects created via Object.create(null) or from different realms. This is a pre-existing pattern in the codebase so not introduced by this PR, but worth noting if async config resolution encounters such objects.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/util.js`:
- Line 328: The forEach callback is implicitly returning the value of
defer.resolve(), causing a lint warning; update the deferred.forEach callback so
it uses a block body that calls defer.resolve() without returning it (i.e.,
replace the concise arrow callback with a block-style arrow function that simply
invokes defer.resolve();), targeting the deferred array iteration where
deferred.forEach((defer) => defer.resolve()) is used to ensure no value is
returned from the callback.

…fig()

This should answer node-config#740 and also opens the door for migrating to ESM modules.

This does not at present address the issue of async.
And move some integration tests to unit tests.

Prep work for adding async support to defer()
This will probably be better overall for anyone doing an incremental
migration since the new code will run last.
https://github.com/node-config/node-config/wiki/Future-Compatibility

"If it's possible to detect your application using a future incompatibility,
a one-time warning per application run will be placed onto console.error for
each type of incompatibility.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@test/3-config/default.js`:
- Line 1: The file imports setTimeout from 'node:timers/promises' but never uses
it; remove the unused import to eliminate the dead dependency and any linter
warnings — delete the "const { setTimeout } = require('node:timers/promises');"
statement (or replace it with needed imports if other timers are required) so no
reference to setTimeout remains in the file.
🧹 Nitpick comments (2)
lib/util.js (1)

710-717: errorOnce implementation is fine; minor style nit on new Error.

new Error; (line 713) works but new Error() is more conventional. Error.captureStackTrace is V8-specific but appropriate for a Node.js-only library.

test/0-util.js (1)

10-15: Duplicate import of deferConfig.

Line 11 imports deferConfig as defer, and line 15 imports deferConfig again under its original name. Consider consolidating into one import and using both names.

-const {deferConfig: defer} = require("../defer");
+const {deferConfig: defer, deferConfig} = require("../defer");
 const {asyncConfig} = require("../async");
 const util = require('../lib/util.js').Util;
 const Load = require('../lib/util.js').Load;
-const deferConfig = require('../defer').deferConfig;

justThis: defer(function () {
return "Welcome to this "+this.siteTitle;
}),
const { setTimeout } = require('node:timers/promises');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unused import: setTimeout from node:timers/promises is never referenced in this file.

-const { setTimeout } = require('node:timers/promises');
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { setTimeout } = require('node:timers/promises');
🤖 Prompt for AI Agents
In `@test/3-config/default.js` at line 1, The file imports setTimeout from
'node:timers/promises' but never uses it; remove the unused import to eliminate
the dead dependency and any linter warnings — delete the "const { setTimeout } =
require('node:timers/promises');" statement (or replace it with needed imports
if other timers are required) so no reference to setTimeout remains in the file.

Copy link
Collaborator

@markstos markstos left a comment

Choose a reason for hiding this comment

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

Regarding the "Forward-Compatibilty Clause", I had forgotten about that, as the world was largely standardized on semver for managing expectations about compatibility.

QA Log

  • Reviewed diff

@jdmarshall jdmarshall merged commit bd4d95d into node-config:master Feb 12, 2026
2 checks passed
@jdmarshall
Copy link
Collaborator Author

I did not end up exposing 'raw' with this. I suppose I probably should?

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.

Cannot use defer with esbuild (fix: export defer from config)

2 participants