feat(config): auto-detect supported config files in the working directory#901
feat(config): auto-detect supported config files in the working directory#901AysajanE wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Tests cover per-extension auto-detection, priority when multiple files exist, explicit-path precedence, and probing all candidates when none exist. Loader tests add integration checks that Reviewed by Cursor Bugbot for commit 662687f. Bugbot is set up for automated code reviews on this repo. Configure here. |
| '@node-core/doc-kit': patch | ||
| --- | ||
|
|
||
| Automatically load `doc-kit.config.mjs` from the current working directory. |
There was a problem hiding this comment.
What about:
.config.js
.config.cjs
.config.json?
There was a problem hiding this comment.
their aren't supported by the code.
IMO we should also support ts, cts, mts as types strippable code
| */ | ||
| export const createRunConfiguration = async options => { | ||
| const config = await loadConfigFile(options.configFile); | ||
| const defaultConfigFile = join(process.cwd(), 'doc-kit.config.mjs'); | ||
| const configFile = | ||
| options.configFile ?? | ||
| (existsSync(defaultConfigFile) ? defaultConfigFile : undefined); | ||
| const config = await loadConfigFile(configFile); |
There was a problem hiding this comment.
the logic here is wrong you should do something like this:
resolve configuration file:
if --config option is provided:
use the provided file
else:
search for a configuration file named:
doc-kit.config.js
doc-kit.config.cjs
doc-kit.config.mjs
doc-kit.config.ts
doc-kit.config.cts
doc-kit.config.mts
use the first file that exists (following a defined priority)
pass the resolved path to loadConfigFile()
There was a problem hiding this comment.
Thanks — understood. I will replace the single .mjs probe with explicit --config-file precedence followed by discovery across the six names you listed, and add coverage for every extension plus the multiple-files case. Before I update the branch, should the order in your example (.js, .cjs, .mjs, .ts, .cts, .mts) also be the precedence when more than one exists? I will leave .json out unless you want it included.
AugustinMauroy
left a comment
There was a problem hiding this comment.
adding blocker since feature isn't correctly covered.
thanks @AysajanE for you effort here
Signed-off-by: Aysajan Eziz <aeziz@northset.ai>
|
Thanks for the specific direction, @AugustinMauroy and @avivkeller. I have updated and rebased the branch to cover the full supported configuration-file set. When --config-file is omitted, discovery now checks this exact priority order:
The first existing file wins. An explicit configFile still bypasses discovery, JSON is not included because it is unsupported, and the no-file behavior remains unchanged. Coverage now includes:
Local verification on current upstream main b9750dc:
I also checked the still-open dynamic-configuration PR #905. It touches the same configuration area but implements dynamic default values, so there is no semantic overlap with this file-discovery correction at present. The updated head is 662687f. Could you please take another look when CI is ready? |
Problem
doc-kit generateonly loads configuration when--config-fileis passed explicitly —loadConfigFilereturns{}otherwise. As raised in #897, most config-driven tools also detect a conventional config file automatically, so a validdoc-kit.config.mjssitting in the project root is silently ignored today. That is an easy footgun for anyone who assumes the file is picked up.Change
createRunConfiguration, when no--config-fileis provided, auto-detect adoc-kit.config.mjsin the current working directory and load it through the existing loader. An explicit--config-filestill takes precedence, and when no file is present the configuration falls back to empty — unchanged behavior. No new dependency; a tight happy path, as the issue suggests.Checks (Node 22)
node --test --experimental-test-module-mocks src/utils/configuration/__tests__/index.test.mjs— 17 passing, including the new cases.Fixes #897
Disclosure: this change was prepared with AI assistance and reviewed by me before submitting.
I ran the check(s) above in a network-isolated container and published a signed, re-runnable
record of that run, verifiable via GitHub artifact attestation:
https://northset-oss.github.io/verification-pilot/. Contributor self-run, not a maintainer verification.