Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: resolve config modules with ESM createRequire (#1082)
ES modules do not have require() available, we must construct it first.
This fixes the CLI when passing `--config my-config-package`, broken in
v12.0.0.

The tests didn't catch this because Jest still doesn't support mocking
ESM, and there's really no way to write a test for this right now.
  • Loading branch information
evocateur committed Jan 7, 2022
1 parent 893f3d7 commit f9f6538
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 14 deletions.
7 changes: 6 additions & 1 deletion jest.config.js
@@ -1,6 +1,11 @@
const config = {
collectCoverage: true,
collectCoverageFrom: ['lib/**/*.js'],
collectCoverageFrom: [
'lib/**/*.js',
// Avoid ESM import.meta parse error.
// (Can't measure coverage anyway, it's always mocked)
'!lib/resolveConfig.js',
],
moduleDirectories: ['node_modules'],
setupFiles: ['./testSetup.js'],
snapshotSerializers: ['jest-snapshot-serializer-ansi'],
Expand Down
10 changes: 2 additions & 8 deletions lib/loadConfig.js
Expand Up @@ -6,6 +6,8 @@ import debug from 'debug'
import { lilconfig } from 'lilconfig'
import YAML from 'yaml'

import { resolveConfig } from './resolveConfig.js'

const debugLog = debug('lint-staged:loadConfig')

/**
Expand Down Expand Up @@ -49,14 +51,6 @@ const loaders = {
noExt: yamlParse,
}

const resolveConfig = (configPath) => {
try {
return require.resolve(configPath)
} catch {
return configPath
}
}

/**
* @param {object} options
* @param {string} [options.configPath] - Explicit path to a config file
Expand Down
15 changes: 15 additions & 0 deletions lib/resolveConfig.js
@@ -0,0 +1,15 @@
import { createRequire } from 'module'

/**
* require() does not exist for ESM, so we must create it to use require.resolve().
* @see https://nodejs.org/api/module.html#modulecreaterequirefilename
*/
const require = createRequire(import.meta.url)

export function resolveConfig(configPath) {
try {
return require.resolve(configPath)
} catch {
return configPath
}
}
10 changes: 10 additions & 0 deletions test/index.spec.js
Expand Up @@ -28,6 +28,16 @@ jest.mock('url', () => ({

jest.mock('../lib/getStagedFiles')
jest.mock('../lib/gitWorkflow')
jest.mock('../lib/resolveConfig', () => ({
/** Unfortunately necessary due to non-ESM tests. */
resolveConfig: (configPath) => {
try {
return require.resolve(configPath)
} catch {
return configPath
}
},
}))
jest.mock('../lib/validateOptions', () => ({
validateOptions: jest.fn().mockImplementation(async () => {}),
}))
Expand Down
11 changes: 11 additions & 0 deletions test/index2.spec.js
Expand Up @@ -4,6 +4,17 @@ import { Listr } from 'listr2'
import makeConsoleMock from 'consolemock'

jest.mock('listr2')
jest.mock('../lib/resolveConfig', () => ({
/** Unfortunately necessary due to non-ESM tests. */
resolveConfig: (configPath) => {
try {
return require.resolve(configPath)
} catch {
return configPath
}
},
}))

jest.mock('../lib/resolveGitRepo')

import lintStaged from '../lib/index'
Expand Down
27 changes: 22 additions & 5 deletions test/integration.test.js
Expand Up @@ -7,6 +7,16 @@ import normalize from 'normalize-path'

jest.unmock('lilconfig')
jest.unmock('execa')
jest.mock('../lib/resolveConfig', () => ({
/** Unfortunately necessary due to non-ESM tests. */
resolveConfig: (configPath) => {
try {
return require.resolve(configPath)
} catch {
return configPath
}
},
}))

import { execGit as execGitBase } from '../lib/execGit'
import lintStaged from '../lib/index'
Expand Down Expand Up @@ -88,6 +98,9 @@ describe('lint-staged', () => {

const globalConsoleTemp = console

// Tests should be resilient to `git config init.defaultBranch` that is _not_ "master"
let defaultBranchName = 'UNSET'

describe('lint-staged', () => {
beforeAll(() => {
console = makeConsoleMock()
Expand All @@ -104,6 +117,10 @@ describe('lint-staged', () => {
await appendFile('README.md', '# Test\n')
await execGit(['add', 'README.md'])
await execGit(['commit', '-m initial commit'])

if (defaultBranchName === 'UNSET') {
defaultBranchName = await execGit(['rev-parse', '--abbrev-ref', 'HEAD'])
}
})

afterEach(async () => {
Expand Down Expand Up @@ -451,7 +468,7 @@ describe('lint-staged', () => {
await gitCommit(fixJsConfig, ['-m commit a'])
expect(await readFile('test.js')).toEqual(fileInBranchA)

await execGit(['checkout', 'master'])
await execGit(['checkout', defaultBranchName])

// Create another branch
await execGit(['checkout', '-b', 'branch-b'])
Expand All @@ -461,7 +478,7 @@ describe('lint-staged', () => {
expect(await readFile('test.js')).toEqual(fileInBranchBFixed)

// Merge first branch
await execGit(['checkout', 'master'])
await execGit(['checkout', defaultBranchName])
await execGit(['merge', 'branch-a'])
expect(await readFile('test.js')).toEqual(fileInBranchA)
expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('commit a')
Expand Down Expand Up @@ -512,7 +529,7 @@ describe('lint-staged', () => {
await gitCommit(fixJsConfig, ['-m commit a'])
expect(await readFile('test.js')).toEqual(fileInBranchA)

await execGit(['checkout', 'master'])
await execGit(['checkout', defaultBranchName])

// Create another branch
await execGit(['checkout', '-b', 'branch-b'])
Expand All @@ -522,7 +539,7 @@ describe('lint-staged', () => {
expect(await readFile('test.js')).toEqual(fileInBranchBFixed)

// Merge first branch
await execGit(['checkout', 'master'])
await execGit(['checkout', defaultBranchName])
await execGit(['merge', 'branch-a'])
expect(await readFile('test.js')).toEqual(fileInBranchA)
expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('commit a')
Expand Down Expand Up @@ -1073,7 +1090,7 @@ describe('lintStaged', () => {
await execGit(['add', 'test.js'], { cwd })

await expect(execGit(['log', '-1'], { cwd })).rejects.toThrowErrorMatchingInlineSnapshot(
`"fatal: your current branch 'master' does not have any commits yet"`
`"fatal: your current branch '${defaultBranchName}' does not have any commits yet"`
)

await gitCommit({
Expand Down
11 changes: 11 additions & 0 deletions test/loadConfig.spec.js
@@ -1,3 +1,14 @@
jest.mock('../lib/resolveConfig', () => ({
/** Unfortunately necessary due to non-ESM tests. */
resolveConfig: (configPath) => {
try {
return require.resolve(configPath)
} catch {
return configPath
}
},
}))

import { dynamicImport } from '../lib/loadConfig.js'

describe('dynamicImport', () => {
Expand Down

0 comments on commit f9f6538

Please sign in to comment.