Skip to content

Commit 68f4f09

Browse files
jobersteinjoberstein
authored
Fix/3768: Consider ESM when selecting cosmiconfig loaders (#3776)
* feat(load): use cosmiconfig-typescript-loader v5 to remove ts-node dependency for @commitlint/load * fix(load): add support for async loaders for ESM applications * chore(load): simplify loader selection for js/cjs files * docs: remove node version restriction from mjs config
1 parent f9361f6 commit 68f4f09

32 files changed

+223
-62
lines changed

‎@commitlint/load/fixtures/basic-config/.commitlintrc

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
formatter: '@commitlint/format'
2+
rules:
3+
zero: [0, 'never']
4+
one: [1, 'always']
5+
two: [2, 'never']

‎@commitlint/load/fixtures/basic-config/.commitlintrc.cjs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/.commitlintrc.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/.commitlintrc.json

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"formatter": "@commitlint/format",
3+
"rules": {
4+
"zero": [0, "never"],
5+
"one": [1, "always"],
6+
"two": [2, "never"]
7+
}
8+
}

‎@commitlint/load/fixtures/basic-config/.commitlintrc.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
formatter: '@commitlint/format'
2+
rules:
3+
zero: [0, 'never']
4+
one: [1, 'always']
5+
two: [2, 'never']

‎@commitlint/load/fixtures/basic-config/.commitlintrc.yml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
formatter: '@commitlint/format'
2+
rules:
3+
zero: [0, 'never']
4+
one: [1, 'always']
5+
two: [2, 'never']

‎@commitlint/load/fixtures/basic-config/commitlint.config.cjs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/commitlint.config.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/esm/.commitlintrc.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/esm/.commitlintrc.mjs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/esm/commitlint.config.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-config/esm/commitlint.config.mjs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default {
2+
formatter: '@commitlint/format',
3+
rules: {
4+
zero: [0, 'never'],
5+
one: [1, 'always'],
6+
two: [2, 'never'],
7+
},
8+
};

‎@commitlint/load/fixtures/basic-template/package.json

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "load-test-js"
3+
}

‎@commitlint/load/fixtures/config/package.json

-13
This file was deleted.

‎@commitlint/load/fixtures/config/.commitlintrc ‎@commitlint/load/fixtures/extends-config/.commitlintrc

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.cjs ‎@commitlint/load/fixtures/extends-config/.commitlintrc.cjs

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.js ‎@commitlint/load/fixtures/extends-config/.commitlintrc.js

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.json ‎@commitlint/load/fixtures/extends-config/.commitlintrc.json

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.yaml ‎@commitlint/load/fixtures/extends-config/.commitlintrc.yaml

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.yml ‎@commitlint/load/fixtures/extends-config/.commitlintrc.yml

File renamed without changes.

‎@commitlint/load/fixtures/config/commitlint.config.cjs ‎@commitlint/load/fixtures/extends-config/commitlint.config.cjs

File renamed without changes.

‎@commitlint/load/fixtures/config/commitlint.config.js ‎@commitlint/load/fixtures/extends-config/commitlint.config.js

File renamed without changes.

‎@commitlint/load/fixtures/config/.commitlintrc.mjs ‎@commitlint/load/fixtures/extends-config/esm/.commitlintrc.js

File renamed without changes.

‎@commitlint/load/fixtures/config/commitlint.config.mjs ‎@commitlint/load/fixtures/extends-config/esm/.commitlintrc.mjs

File renamed without changes.

‎@commitlint/load/fixtures/extends-config/esm/commitlint.config.js

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export default {
2+
extends: ['./first-extended'],
3+
rules: {
4+
zero: [0, 'never'],
5+
},
6+
};

‎@commitlint/load/fixtures/extends-config/esm/commitlint.config.mjs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export default {
2+
extends: ['./first-extended'],
3+
rules: {
4+
zero: [0, 'never'],
5+
},
6+
};

‎@commitlint/load/fixtures/recursive-extends-js-template/first-extended/index.js ‎@commitlint/load/fixtures/extends-js-template/first-extended/index.js

File renamed without changes.

‎@commitlint/load/fixtures/recursive-extends-js-template/first-extended/second-extended/index.js ‎@commitlint/load/fixtures/extends-js-template/first-extended/second-extended/index.js

File renamed without changes.

‎@commitlint/load/fixtures/extends-js-template/package.json

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "load-test-js"
3+
}

‎@commitlint/load/src/load.test.ts

+92-25
Original file line numberDiff line numberDiff line change
@@ -188,41 +188,108 @@ test('respects cwd option', async () => {
188188
});
189189
});
190190

191-
const mjsConfigFiles = isDynamicAwaitSupported()
192-
? ['commitlint.config.mjs', '.commitlintrc.mjs']
193-
: [];
191+
describe.each([['basic'], ['extends']])('%s config', (template) => {
192+
const isExtendsTemplate = template === 'extends';
194193

195-
test.each(
196-
[
194+
const configFiles = [
197195
'commitlint.config.cjs',
198196
'commitlint.config.js',
197+
'commitlint.config.mjs',
199198
'package.json',
200199
'.commitlintrc',
201200
'.commitlintrc.cjs',
202201
'.commitlintrc.js',
203202
'.commitlintrc.json',
203+
'.commitlintrc.mjs',
204204
'.commitlintrc.yml',
205205
'.commitlintrc.yaml',
206-
...mjsConfigFiles,
207-
].map((configFile) => [configFile])
208-
)('recursive extends with %s', async (configFile) => {
209-
const cwd = await gitBootstrap(`fixtures/recursive-extends-js-template`);
210-
const configPath = path.join(__dirname, `../fixtures/config/${configFile}`);
211-
const config = readFileSync(configPath);
212-
213-
writeFileSync(path.join(cwd, configFile), config);
214-
215-
const actual = await load({}, {cwd});
216-
217-
expect(actual).toMatchObject({
218-
formatter: '@commitlint/format',
219-
extends: ['./first-extended'],
220-
plugins: {},
221-
rules: {
222-
zero: [0, 'never'],
223-
one: [1, 'always'],
224-
two: [2, 'never'],
225-
},
206+
];
207+
208+
const configTestCases = [
209+
...configFiles
210+
.filter((filename) => !filename.endsWith('.mjs'))
211+
.map((filename) => ({filename, isEsm: false})),
212+
...configFiles
213+
.filter((filename) =>
214+
['.mjs', '.js'].some((ext) => filename.endsWith(ext))
215+
)
216+
.map((filename) => ({filename, isEsm: true})),
217+
];
218+
219+
const getConfigContents = ({
220+
filename,
221+
isEsm,
222+
}): string | NodeJS.ArrayBufferView => {
223+
if (filename === 'package.json') {
224+
const configPath = path.join(
225+
__dirname,
226+
`../fixtures/${template}-config/.commitlintrc.json`
227+
);
228+
const commitlint = JSON.parse(
229+
readFileSync(configPath, {encoding: 'utf-8'})
230+
);
231+
return JSON.stringify({commitlint});
232+
} else {
233+
const filePath = ['..', 'fixtures', `${template}-config`, filename];
234+
235+
if (isEsm) {
236+
filePath.splice(3, 0, 'esm');
237+
}
238+
239+
const configPath = path.join(__dirname, filePath.join('/'));
240+
return readFileSync(configPath);
241+
}
242+
};
243+
244+
const esmBootstrap = (cwd: string) => {
245+
const packageJsonPath = path.join(cwd, 'package.json');
246+
const packageJSON = JSON.parse(
247+
readFileSync(packageJsonPath, {encoding: 'utf-8'})
248+
);
249+
250+
writeFileSync(
251+
packageJsonPath,
252+
JSON.stringify({
253+
...packageJSON,
254+
type: 'module',
255+
})
256+
);
257+
};
258+
259+
const templateFolder = [template, isExtendsTemplate ? 'js' : '', 'template']
260+
.filter((elem) => elem)
261+
.join('-');
262+
263+
it.each(
264+
configTestCases
265+
// Skip ESM tests for the extends suite until resolve-extends supports ESM
266+
.filter(({isEsm}) => template !== 'extends' || !isEsm)
267+
// Skip ESM tests if dynamic await is not supported; Jest will crash with a seg fault error
268+
.filter(({isEsm}) => isDynamicAwaitSupported() || !isEsm)
269+
)('$filename, ESM: $isEsm', async ({filename, isEsm}) => {
270+
const cwd = await gitBootstrap(`fixtures/${templateFolder}`);
271+
272+
if (isEsm) {
273+
esmBootstrap(cwd);
274+
}
275+
276+
writeFileSync(
277+
path.join(cwd, filename),
278+
getConfigContents({filename, isEsm})
279+
);
280+
281+
const actual = await load({}, {cwd});
282+
283+
expect(actual).toMatchObject({
284+
formatter: '@commitlint/format',
285+
extends: isExtendsTemplate ? ['./first-extended'] : [],
286+
plugins: {},
287+
rules: {
288+
zero: [0, 'never'],
289+
one: [1, 'always'],
290+
two: [2, 'never'],
291+
},
292+
});
226293
});
227294
});
228295

‎@commitlint/load/src/utils/load-config.ts

+24-22
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import {
22
cosmiconfig,
33
defaultLoadersSync,
4-
Options,
54
type Loader,
5+
defaultLoaders,
66
} from 'cosmiconfig';
77
import {TypeScriptLoader} from 'cosmiconfig-typescript-loader';
8+
import {existsSync, readFileSync} from 'fs';
89
import path from 'path';
910

1011
export interface LoadConfigResult {
@@ -27,7 +28,12 @@ export async function loadConfig(
2728
return tsLoaderInstance(...args);
2829
};
2930

30-
const {searchPlaces, loaders} = getDynamicAwaitConfig();
31+
// If dynamic await is supported (Node >= v20.8.0) or directory uses ESM, support
32+
// async js/cjs loaders (dynamic import). Otherwise, use synchronous js/cjs loaders.
33+
const loaders =
34+
isDynamicAwaitSupported() || isEsmModule(cwd)
35+
? defaultLoaders
36+
: defaultLoadersSync;
3137

3238
const explorer = cosmiconfig(moduleName, {
3339
searchPlaces: [
@@ -40,22 +46,22 @@ export async function loadConfig(
4046
`.${moduleName}rc.yml`,
4147
`.${moduleName}rc.js`,
4248
`.${moduleName}rc.cjs`,
49+
`.${moduleName}rc.mjs`,
4350
`${moduleName}.config.js`,
4451
`${moduleName}.config.cjs`,
52+
`${moduleName}.config.mjs`,
4553

4654
// files supported by TypescriptLoader
4755
`.${moduleName}rc.ts`,
4856
`.${moduleName}rc.cts`,
4957
`${moduleName}.config.ts`,
5058
`${moduleName}.config.cts`,
51-
52-
...(searchPlaces || []),
5359
],
5460
loaders: {
5561
'.ts': tsLoader,
5662
'.cts': tsLoader,
57-
58-
...(loaders || {}),
63+
'.cjs': loaders['.cjs'],
64+
'.js': loaders['.js'],
5965
},
6066
});
6167

@@ -71,7 +77,7 @@ export async function loadConfig(
7177
return null;
7278
}
7379

74-
// See the following issues for more context:
80+
// See the following issues for more context, contributing to failing Jest tests:
7581
// - Issue: https://github.com/nodejs/node/issues/40058
7682
// - Resolution: https://github.com/nodejs/node/pull/48510 (Node v20.8.0)
7783
export const isDynamicAwaitSupported = () => {
@@ -83,18 +89,14 @@ export const isDynamicAwaitSupported = () => {
8389
return major >= 20 && minor >= 8;
8490
};
8591

86-
// If dynamic await is supported (Node >= v20.8.0), support mjs config.
87-
// Otherwise, don't support mjs and use synchronous js/cjs loaders.
88-
export const getDynamicAwaitConfig = (): Partial<Options> =>
89-
isDynamicAwaitSupported()
90-
? {
91-
searchPlaces: [`.${moduleName}rc.mjs`, `${moduleName}.config.mjs`],
92-
loaders: {},
93-
}
94-
: {
95-
searchPlaces: [],
96-
loaders: {
97-
'.cjs': defaultLoadersSync['.cjs'],
98-
'.js': defaultLoadersSync['.js'],
99-
},
100-
};
92+
// Is the given directory set up to use ESM (ECMAScript Modules)?
93+
export const isEsmModule = (cwd: string) => {
94+
const packagePath = path.join(cwd, 'package.json');
95+
96+
if (!existsSync(packagePath)) {
97+
return false;
98+
}
99+
100+
const packageJSON = readFileSync(packagePath, {encoding: 'utf-8'});
101+
return JSON.parse(packageJSON)?.type === 'module';
102+
};

‎README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,12 @@ Check the [husky documentation](https://typicode.github.io/husky/#/?id=manual) o
142142
- `.commitlintrc.yml`
143143
- `.commitlintrc.js`
144144
- `.commitlintrc.cjs`
145-
- `.commitlintrc.mjs` (Node >= v20.8.0)
145+
- `.commitlintrc.mjs`
146146
- `.commitlintrc.ts`
147147
- `.commitlintrc.cts`
148148
- `commitlint.config.js`
149149
- `commitlint.config.cjs`
150-
- `commitlint.config.mjs` (Node >= v20.8.0)
150+
- `commitlint.config.mjs`
151151
- `commitlint.config.ts`
152152
- `commitlint.config.cts`
153153
- `commitlint` field in `package.json`

0 commit comments

Comments
 (0)