Skip to content

Commit 8e626ca

Browse files
committed
feat(no-sync)!: move ts-declaration-location to peerDependencies
- Add `typescript` as an optional peer dependency for the `n/no-sync` rule, used on `lib/util/get-full-type-name.js`. - Set `ts-declaration-location` as an optional peer dependency for the `n/no-sync` rule. - Move `ts-declaration-location` to `devDependencies` for development and testing. - Update the `n/no-sync` documentation to refer the updated peer dependency requirement. - Update `n/no-sync` rule to conditionally require `ts-declaration-location` only when advanced ignores are used. - Remove unused eslint-disable directive n/no-unpublished-require on `lib/util/get-full-type-name.js`, as `typescript` is now an optional peer dependency. - Add tests to ensure the rule throws an error when `ts-declaration-location` or TypeScript parser services are not available. - Add `sinon` as a dev dependency, used for mocking in tests.
1 parent 8185617 commit 8e626ca

File tree

5 files changed

+208
-18
lines changed

5 files changed

+208
-18
lines changed

docs/rules/no-sync.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ Examples of **correct** code for this rule with the `{ ignores: ['readFileSync']
7979
fs.readFileSync(somePath);
8080
```
8181

82+
> [!WARNING]
83+
> Advanced `ignores` options (object specifiers) require TypeScript and the [`ts-declaration-location`](https://www.npmjs.com/package/ts-declaration-location) package. This package is an **optional peer dependency** for the `n/no-sync` rule. If you want to use advanced TypeScript-based ignores, please install it in your project:
84+
>
85+
> ```sh
86+
> npm install --save-dev ts-declaration-location
87+
> ```
88+
8289
##### Advanced (TypeScript only)
8390
8491
You can provide a list of specifiers to ignore. Specifiers are typed as follows:
@@ -102,6 +109,9 @@ type Specifier =
102109
}
103110
```
104111
112+
> [!NOTE]
113+
> To use advanced TypeScript-based ignores, you must have `ts-declaration-location` installed as a dependency in your project.
114+
105115
###### From a file
106116

107117
Examples of **correct** code for this rule with the ignore file specifier:

lib/rules/no-sync.js

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,6 @@ let typeMatchesSpecifier =
88
/** @type {import('ts-declaration-location').default | undefined} */
99
(undefined)
1010

11-
try {
12-
typeMatchesSpecifier =
13-
/** @type {import('ts-declaration-location').default} */ (
14-
/** @type {unknown} */ (require("ts-declaration-location"))
15-
)
16-
17-
// eslint-disable-next-line no-empty -- Deliberately left empty.
18-
} catch {}
1911
const getTypeOfNode = require("../util/get-type-of-node")
2012
const getParserServices = require("../util/get-parser-services")
2113
const getFullTypeName = require("../util/get-full-type-name")
@@ -124,6 +116,27 @@ module.exports = {
124116
const selector = options.allowAtRootLevel
125117
? selectors.map(selector => `:function ${selector}`)
126118
: selectors
119+
120+
const hasAdvancedIgnores = ignores.some(
121+
ignore => typeof ignore !== "string"
122+
)
123+
124+
// Only require `ts-declaration-location` if needed and not already required.
125+
if (hasAdvancedIgnores) {
126+
try {
127+
typeMatchesSpecifier ||=
128+
/** @type {import('ts-declaration-location').default} */ (
129+
/** @type {unknown} */ (
130+
require("ts-declaration-location")
131+
)
132+
)
133+
} catch {
134+
throw new Error(
135+
'ts-declaration-location not available. Rule "n/no-sync" is configured to use "ignores" option with a non-string value. This requires ts-declaration-location to be available.'
136+
)
137+
}
138+
}
139+
127140
return {
128141
/**
129142
* @param {import('estree').Identifier & {parent: import('estree').Node}} node
@@ -160,12 +173,6 @@ module.exports = {
160173
)
161174
}
162175

163-
if (typeMatchesSpecifier === undefined) {
164-
throw new Error(
165-
'ts-declaration-location not available. Rule "n/no-sync" is configured to use "ignores" option with a non-string value. This requires ts-declaration-location to be available.'
166-
)
167-
}
168-
169176
type =
170177
type === undefined
171178
? getTypeOfNode(node, parserServices)
@@ -177,6 +184,7 @@ module.exports = {
177184
: fullName
178185

179186
if (
187+
typeMatchesSpecifier &&
180188
typeMatchesSpecifier(
181189
parserServices.program,
182190
ignore,

lib/util/get-full-type-name.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
const ts = (() => {
44
try {
5-
// eslint-disable-next-line n/no-unpublished-require
65
return require("typescript")
76
} catch {
87
return null

package.json

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,17 @@
1414
"types/index.d.ts"
1515
],
1616
"peerDependencies": {
17-
"eslint": ">=8.23.0"
17+
"eslint": ">=8.23.0",
18+
"ts-declaration-location": "^1.0.6",
19+
"typescript": ">=4.8.4"
20+
},
21+
"peerDependenciesMeta": {
22+
"ts-declaration-location": {
23+
"optional": true
24+
},
25+
"typescript": {
26+
"optional": true
27+
}
1828
},
1929
"dependencies": {
2030
"@eslint-community/eslint-utils": "^4.5.0",
@@ -24,8 +34,7 @@
2434
"globals": "^15.11.0",
2535
"ignore": "^5.3.2",
2636
"minimatch": "^9.0.5",
27-
"semver": "^7.6.3",
28-
"ts-declaration-location": "^1.0.6"
37+
"semver": "^7.6.3"
2938
},
3039
"devDependencies": {
3140
"@eslint/js": "^9.14.0",
@@ -51,6 +60,8 @@
5160
"punycode": "^2.3.1",
5261
"release-it": "^17.10.0",
5362
"rimraf": "^5.0.10",
63+
"sinon": "^21.0.0",
64+
"ts-declaration-location": "^1.0.6",
5465
"ts-ignore-import": "^4.0.1",
5566
"type-fest": "^4.26.1",
5667
"typescript": "~5.6"

tests/lib/rules/no-sync.js

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
"use strict"
66

77
const { RuleTester, TsRuleTester } = require("#test-helpers")
8+
const Module = require("node:module")
9+
const assert = require("node:assert")
810
const rule = require("../../../lib/rules/no-sync")
11+
const sinon = require("sinon")
912

1013
new RuleTester().run("no-sync", rule, {
1114
valid: [
@@ -319,3 +322,162 @@ fooSync();
319322
},
320323
],
321324
})
325+
326+
describe("no-sync rule with missing dependencies", () => {
327+
const originalRequire = module.require
328+
let mockRequire
329+
let originalModules = {}
330+
331+
/**
332+
* Helper function to mock a module.
333+
* - Assigns a module object to `require.cache` with the provided mock exports to prevent TypeScript errors when the module is required, as TypeScript expects the module to exist in the cache with an 'exports' property.
334+
*
335+
* @param {string} modulePath - The path to the module.
336+
* @param {*} mockExports - The mock exports to use.
337+
*/
338+
function mockModule(modulePath, mockExports) {
339+
const resolvedPath = require.resolve(modulePath)
340+
341+
// Store original module if not already stored.
342+
if (!originalModules[resolvedPath] && require.cache[resolvedPath]) {
343+
originalModules[resolvedPath] = require.cache[resolvedPath]
344+
}
345+
346+
require.cache[resolvedPath] = { exports: mockExports }
347+
}
348+
349+
/**
350+
* Helper to test rule behavior with missing dependencies.
351+
* - Sets mocks for each dependency based on options.
352+
*
353+
* @param {object} options - Test options.
354+
* @param {boolean} options.mockTsDeclarationLocation - Whether to mock `ts-declaration-location` as missing.
355+
* @param {boolean} options.mockTypeScriptServices - Whether to mock TypeScript services as missing.
356+
* @param {RegExp} options.expectedError - The expected error message pattern.
357+
*/
358+
function testWithMissingDependency(options) {
359+
const mockModules = {}
360+
361+
if (options.mockTsDeclarationLocation) {
362+
mockModules["ts-declaration-location"] = {
363+
throws: new Error(
364+
"Cannot find module 'ts-declaration-location'"
365+
),
366+
}
367+
}
368+
369+
// Stub for the require function.
370+
mockRequire = sinon
371+
.stub(Module.prototype, "require")
372+
.callsFake(function (id) {
373+
if (mockModules[id] && mockModules[id].throws) {
374+
throw mockModules[id].throws
375+
}
376+
return originalRequire.apply(this, arguments)
377+
})
378+
379+
if (options.mockTypeScriptServices) {
380+
const mockGetParserServices = function () {
381+
return null
382+
}
383+
384+
mockModule(
385+
"../../../lib/util/get-parser-services",
386+
mockGetParserServices
387+
)
388+
}
389+
390+
// Directly create and test the rule.
391+
const rule = require("../../../lib/rules/no-sync")
392+
393+
// Context with required fields to satisfy TypeScript parser requirements.
394+
const context = {
395+
options: [
396+
{
397+
ignores: [
398+
{
399+
from: "file",
400+
},
401+
],
402+
},
403+
],
404+
}
405+
406+
// Node that triggers the rule.
407+
const node = {
408+
name: "fooSync",
409+
}
410+
411+
// Test if the rule throws the expected error.
412+
let errorThrown = false
413+
try {
414+
const ruleListener = rule.create(context)
415+
const selectors = Object.keys(ruleListener)
416+
417+
for (const selector of selectors) {
418+
try {
419+
if (typeof ruleListener[selector] === "function") {
420+
ruleListener[selector](node)
421+
}
422+
} catch (e) {
423+
if (e.message.match(options.expectedError)) {
424+
errorThrown = true
425+
break
426+
}
427+
}
428+
}
429+
} catch (e) {
430+
if (e.message.match(options.expectedError)) {
431+
errorThrown = true
432+
} else {
433+
throw e
434+
}
435+
}
436+
437+
assert.ok(
438+
errorThrown,
439+
`Expected error matching ${options.expectedError} was not thrown`
440+
)
441+
}
442+
443+
beforeEach(() => {
444+
delete require.cache[require.resolve("../../../lib/rules/no-sync")]
445+
delete require.cache[
446+
require.resolve("../../../lib/util/get-parser-services")
447+
]
448+
})
449+
450+
afterEach(() => {
451+
if (mockRequire && typeof mockRequire.restore === "function") {
452+
mockRequire.restore()
453+
}
454+
455+
sinon.restore()
456+
module.require = originalRequire
457+
})
458+
459+
it("should throw if `ts-declaration-location` is not installed", function () {
460+
testWithMissingDependency({
461+
mockTsDeclarationLocation: true,
462+
mockTypeScriptServices: false,
463+
expectedError: /ts-declaration-location not available/,
464+
})
465+
})
466+
467+
it("should throw if TypeScript parser services are not available", function () {
468+
testWithMissingDependency({
469+
mockTsDeclarationLocation: false,
470+
mockTypeScriptServices: true,
471+
expectedError: /TypeScript parser services not available/,
472+
})
473+
})
474+
475+
it("should throw if both `ts-declaration-location` and `typescript` are not available", function () {
476+
testWithMissingDependency({
477+
mockTsDeclarationLocation: true,
478+
mockTypeScriptServices: true,
479+
expectedError:
480+
/TypeScript parser services not available|ts-declaration-location not available/,
481+
})
482+
})
483+
})

0 commit comments

Comments
 (0)