Skip to content

feat(no-sync)!: move ts-declaration-location to peerDependencies #451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(no-sync)!: move ts-declaration-location to peerDependencies #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/rules/no-sync.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ Examples of **correct** code for this rule with the `{ ignores: ['readFileSync']
fs.readFileSync(somePath);
```

> [!WARNING]
> 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:
>
> ```sh
> npm install --save-dev ts-declaration-location
> ```

##### Advanced (TypeScript only)

You can provide a list of specifiers to ignore. Specifiers are typed as follows:
Expand All @@ -102,6 +109,9 @@ type Specifier =
}
```

> [!NOTE]
> To use advanced TypeScript-based ignores, you must have `ts-declaration-location` installed as a dependency in your project.

###### From a file

Examples of **correct** code for this rule with the ignore file specifier:
Expand Down
27 changes: 19 additions & 8 deletions lib/rules/no-sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ let typeMatchesSpecifier =
/** @type {import('ts-declaration-location').default | undefined} */
Copy link

@RebeccaStevens RebeccaStevens Jun 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type should now be: /** @type {import('ts-declaration-location').default | undefined | null} */

(undefined)

try {
typeMatchesSpecifier =
/** @type {import('ts-declaration-location').default} */ (
/** @type {unknown} */ (require("ts-declaration-location"))
)

// eslint-disable-next-line no-empty -- Deliberately left empty.
} catch {}
const getTypeOfNode = require("../util/get-type-of-node")
const getParserServices = require("../util/get-parser-services")
const getFullTypeName = require("../util/get-full-type-name")
Expand Down Expand Up @@ -124,6 +116,25 @@ module.exports = {
const selector = options.allowAtRootLevel
? selectors.map(selector => `:function ${selector}`)
: selectors

const hasAdvancedIgnores = ignores.some(
ignore => typeof ignore !== "string"
)

// Only require `ts-declaration-location` if needed and not already required.
if (hasAdvancedIgnores && typeMatchesSpecifier === undefined) {
try {
typeMatchesSpecifier =
/** @type {import('ts-declaration-location').default} */ (
/** @type {unknown} */ (
require("ts-declaration-location")
)
)
} catch {
// Will throw below if needed.
Copy link

@RebeccaStevens RebeccaStevens Jun 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the catch, add typeMatchesSpecifier = null.

And change this line below if (typeMatchesSpecifier === undefined) { to if (typeMatchesSpecifier == undefined) { (or more explicitly if (typeMatchesSpecifier === undefined || typeMatchesSpecifier === null) {)

Actually, let's just move the throw below up to here:

// Only require `ts-declaration-location` if needed and not already required.
if (hasAdvancedIgnores) {
	if (typeMatchesSpecifier === undefined) {
    try {
      typeMatchesSpecifier =
        /** @type {import('ts-declaration-location').default} */ (
          /** @type {unknown} */ (
            require("ts-declaration-location")
          )
        )
    } catch {
	    typeMatchesSpecifier === null
    }
  } else {
		throw new Error(
      '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.'
    )
  }
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... Or we could move this load down to where the throw below was ...
But now I'm just getting nitpicky.

}
}

return {
/**
* @param {import('estree').Identifier & {parent: import('estree').Node}} node
Expand Down
18 changes: 14 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,17 @@
"types/index.d.ts"
],
"peerDependencies": {
"eslint": ">=8.23.0"
"eslint": ">=8.23.0",
"ts-declaration-location": "^1.0.6",
"typescript": ">=4.8.4"
},
"peerDependenciesMeta": {
"ts-declaration-location": {
"optional": true
},
"typescript": {
"optional": true
}
},
"dependencies": {
"@eslint-community/eslint-utils": "^4.5.0",
Expand All @@ -25,8 +35,7 @@
"globals": "^15.11.0",
"ignore": "^5.3.2",
"minimatch": "^9.0.5",
"semver": "^7.6.3",
"ts-declaration-location": "^1.0.6"
"semver": "^7.6.3"
},
"devDependencies": {
"@eslint/js": "^9.14.0",
Expand All @@ -52,6 +61,7 @@
"punycode": "^2.3.1",
"release-it": "^17.10.0",
"rimraf": "^5.0.10",
"ts-declaration-location": "^1.0.6",
"ts-ignore-import": "^4.0.1",
"type-fest": "^4.26.1",
"typescript": "~5.6"
Expand Down Expand Up @@ -122,4 +132,4 @@
"imports": {
"#test-helpers": "./tests/test-helpers.js"
}
}
}