Skip to content

Commit

Permalink
Fix/3768: Consider ESM when selecting cosmiconfig loaders (#3776)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
joberstein authored Nov 16, 2023
1 parent f9361f6 commit 68f4f09
Show file tree
Hide file tree
Showing 32 changed files with 223 additions and 62 deletions.
5 changes: 5 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
formatter: '@commitlint/format'
rules:
zero: [0, 'never']
one: [1, 'always']
two: [2, 'never']
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"formatter": "@commitlint/format",
"rules": {
"zero": [0, "never"],
"one": [1, "always"],
"two": [2, "never"]
}
}
5 changes: 5 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
formatter: '@commitlint/format'
rules:
zero: [0, 'never']
one: [1, 'always']
two: [2, 'never']
5 changes: 5 additions & 0 deletions @commitlint/load/fixtures/basic-config/.commitlintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
formatter: '@commitlint/format'
rules:
zero: [0, 'never']
one: [1, 'always']
two: [2, 'never']
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/commitlint.config.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/commitlint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/esm/.commitlintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
8 changes: 8 additions & 0 deletions @commitlint/load/fixtures/basic-config/esm/.commitlintrc.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default {
formatter: '@commitlint/format',
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
};
3 changes: 3 additions & 0 deletions @commitlint/load/fixtures/basic-template/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "load-test-js"
}
13 changes: 0 additions & 13 deletions @commitlint/load/fixtures/config/package.json

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default {
extends: ['./first-extended'],
rules: {
zero: [0, 'never'],
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default {
extends: ['./first-extended'],
rules: {
zero: [0, 'never'],
},
};
3 changes: 3 additions & 0 deletions @commitlint/load/fixtures/extends-js-template/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "load-test-js"
}
117 changes: 92 additions & 25 deletions @commitlint/load/src/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,41 +188,108 @@ test('respects cwd option', async () => {
});
});

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

test.each(
[
const configFiles = [
'commitlint.config.cjs',
'commitlint.config.js',
'commitlint.config.mjs',
'package.json',
'.commitlintrc',
'.commitlintrc.cjs',
'.commitlintrc.js',
'.commitlintrc.json',
'.commitlintrc.mjs',
'.commitlintrc.yml',
'.commitlintrc.yaml',
...mjsConfigFiles,
].map((configFile) => [configFile])
)('recursive extends with %s', async (configFile) => {
const cwd = await gitBootstrap(`fixtures/recursive-extends-js-template`);
const configPath = path.join(__dirname, `../fixtures/config/${configFile}`);
const config = readFileSync(configPath);

writeFileSync(path.join(cwd, configFile), config);

const actual = await load({}, {cwd});

expect(actual).toMatchObject({
formatter: '@commitlint/format',
extends: ['./first-extended'],
plugins: {},
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
];

const configTestCases = [
...configFiles
.filter((filename) => !filename.endsWith('.mjs'))
.map((filename) => ({filename, isEsm: false})),
...configFiles
.filter((filename) =>
['.mjs', '.js'].some((ext) => filename.endsWith(ext))
)
.map((filename) => ({filename, isEsm: true})),
];

const getConfigContents = ({
filename,
isEsm,
}): string | NodeJS.ArrayBufferView => {
if (filename === 'package.json') {
const configPath = path.join(
__dirname,
`../fixtures/${template}-config/.commitlintrc.json`
);
const commitlint = JSON.parse(
readFileSync(configPath, {encoding: 'utf-8'})
);
return JSON.stringify({commitlint});
} else {
const filePath = ['..', 'fixtures', `${template}-config`, filename];

if (isEsm) {
filePath.splice(3, 0, 'esm');
}

const configPath = path.join(__dirname, filePath.join('/'));
return readFileSync(configPath);
}
};

const esmBootstrap = (cwd: string) => {
const packageJsonPath = path.join(cwd, 'package.json');
const packageJSON = JSON.parse(
readFileSync(packageJsonPath, {encoding: 'utf-8'})
);

writeFileSync(
packageJsonPath,
JSON.stringify({
...packageJSON,
type: 'module',
})
);
};

const templateFolder = [template, isExtendsTemplate ? 'js' : '', 'template']
.filter((elem) => elem)
.join('-');

it.each(
configTestCases
// Skip ESM tests for the extends suite until resolve-extends supports ESM
.filter(({isEsm}) => template !== 'extends' || !isEsm)
// Skip ESM tests if dynamic await is not supported; Jest will crash with a seg fault error
.filter(({isEsm}) => isDynamicAwaitSupported() || !isEsm)
)('$filename, ESM: $isEsm', async ({filename, isEsm}) => {
const cwd = await gitBootstrap(`fixtures/${templateFolder}`);

if (isEsm) {
esmBootstrap(cwd);
}

writeFileSync(
path.join(cwd, filename),
getConfigContents({filename, isEsm})
);

const actual = await load({}, {cwd});

expect(actual).toMatchObject({
formatter: '@commitlint/format',
extends: isExtendsTemplate ? ['./first-extended'] : [],
plugins: {},
rules: {
zero: [0, 'never'],
one: [1, 'always'],
two: [2, 'never'],
},
});
});
});

Expand Down
46 changes: 24 additions & 22 deletions @commitlint/load/src/utils/load-config.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {
cosmiconfig,
defaultLoadersSync,
Options,
type Loader,
defaultLoaders,
} from 'cosmiconfig';
import {TypeScriptLoader} from 'cosmiconfig-typescript-loader';
import {existsSync, readFileSync} from 'fs';
import path from 'path';

export interface LoadConfigResult {
Expand All @@ -27,7 +28,12 @@ export async function loadConfig(
return tsLoaderInstance(...args);
};

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

const explorer = cosmiconfig(moduleName, {
searchPlaces: [
Expand All @@ -40,22 +46,22 @@ export async function loadConfig(
`.${moduleName}rc.yml`,
`.${moduleName}rc.js`,
`.${moduleName}rc.cjs`,
`.${moduleName}rc.mjs`,
`${moduleName}.config.js`,
`${moduleName}.config.cjs`,
`${moduleName}.config.mjs`,

// files supported by TypescriptLoader
`.${moduleName}rc.ts`,
`.${moduleName}rc.cts`,
`${moduleName}.config.ts`,
`${moduleName}.config.cts`,

...(searchPlaces || []),
],
loaders: {
'.ts': tsLoader,
'.cts': tsLoader,

...(loaders || {}),
'.cjs': loaders['.cjs'],
'.js': loaders['.js'],
},
});

Expand All @@ -71,7 +77,7 @@ export async function loadConfig(
return null;
}

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

// If dynamic await is supported (Node >= v20.8.0), support mjs config.
// Otherwise, don't support mjs and use synchronous js/cjs loaders.
export const getDynamicAwaitConfig = (): Partial<Options> =>
isDynamicAwaitSupported()
? {
searchPlaces: [`.${moduleName}rc.mjs`, `${moduleName}.config.mjs`],
loaders: {},
}
: {
searchPlaces: [],
loaders: {
'.cjs': defaultLoadersSync['.cjs'],
'.js': defaultLoadersSync['.js'],
},
};
// Is the given directory set up to use ESM (ECMAScript Modules)?
export const isEsmModule = (cwd: string) => {
const packagePath = path.join(cwd, 'package.json');

if (!existsSync(packagePath)) {
return false;
}

const packageJSON = readFileSync(packagePath, {encoding: 'utf-8'});
return JSON.parse(packageJSON)?.type === 'module';
};
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,12 @@ Check the [husky documentation](https://typicode.github.io/husky/#/?id=manual) o
- `.commitlintrc.yml`
- `.commitlintrc.js`
- `.commitlintrc.cjs`
- `.commitlintrc.mjs` (Node >= v20.8.0)
- `.commitlintrc.mjs`
- `.commitlintrc.ts`
- `.commitlintrc.cts`
- `commitlint.config.js`
- `commitlint.config.cjs`
- `commitlint.config.mjs` (Node >= v20.8.0)
- `commitlint.config.mjs`
- `commitlint.config.ts`
- `commitlint.config.cts`
- `commitlint` field in `package.json`
Expand Down

0 comments on commit 68f4f09

Please sign in to comment.