fix broken rule, set Angular/Typescript as optionalPeerDeps #32
Conversation
| - It generates broken code cf https://github.com/rdlabo-team/eslint-plugin-rules/issues/5 | ||
| - It applies on irrelevant places cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/1#issuecomment-1980955010> | ||
| - It gets lost with access modifiers cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/4> | ||
| - It generates broken code cf <https://github.com/rdlabo-team/eslint-plugin-rules/issues/5> |
There was a problem hiding this comment.
These were added by my local markdown linter. Looks like it's shorthand for a URL. https://www.markdownguide.org/basic-syntax/#urls-and-email-addresses
Github seems to render them correctly anyway, but it might as well be correct.
| @@ -4,7 +4,7 @@ | |||
| */ | |||
| 'use strict'; | |||
|
|
|||
| const path = require('path'); | |||
| const path = require('node:path'); | |||
There was a problem hiding this comment.
New recommended way to refer to builtin node packages.
| otherFeatureFolders[featureFolderImportedIndex] | ||
| ) | ||
| otherFeatureFolders[featureFolderImportedIndex], | ||
| ), |
There was a problem hiding this comment.
Added and ran prettier
| @@ -5,6 +5,7 @@ | |||
| */ | |||
| 'use strict'; | |||
|
|
|||
| const ts = require('typescript'); | |||
There was a problem hiding this comment.
This missing import caused this rule to error if it were ever used.
| @@ -12,13 +12,14 @@ module.exports = { | |||
| type: 'problem', | |||
| fixable: 'code', | |||
| schema: [], | |||
| messages: { | |||
There was a problem hiding this comment.
This fixes the one existing lint error.
| @@ -18,7 +18,8 @@ | |||
| "devDependencies": { | |||
| "eslint": "8.56.0", | |||
| "eslint-plugin-eslint-plugin": "5.3.0", | |||
| "eslint-plugin-node": "11.1.0" | |||
There was a problem hiding this comment.
eslint-plugin-node was abandoned as of 2020. mysticatea/eslint-plugin-node#300
eslint-plugin-n is the forked replacement.
| @@ -39,7 +40,28 @@ | |||
| "eslint-plugin-react-hooks": "^4.6.0", | |||
| "eslint-plugin-rxjs": ">= 5", | |||
| "eslint-plugin-rxjs-angular": ">= 2", | |||
| "eslint-plugin-simple-import-sort": ">= 12" | |||
| "eslint-plugin-simple-import-sort": ">= 12", | |||
| "typescript": ">= 4" | |||
There was a problem hiding this comment.
Typescript is already included from typescript-eslint, and implicitly depended on. Added it implicitly here.
| @@ -18,7 +18,8 @@ | |||
| "devDependencies": { | |||
| "eslint": "8.56.0", | |||
| "eslint-plugin-eslint-plugin": "5.3.0", | |||
| "eslint-plugin-node": "11.1.0" | |||
| "eslint-plugin-n": "17.10.2", | |||
| "prettier": "3.3.3" | |||
| }, | |||
| "engines": { | |||
There was a problem hiding this comment.
This review came from noticing that a typescript only lib had angular in the node_modules. Since npm made peerDependencies always be installed, installing this plugin causes angular to be included as a transitive dependency through angular-eslint.
It also brings dozens of other packages besides. Setting optional to true in peerDependenciesMeta lets us restrict the version if the package is installed, without forcing it.
Fuzzball24816
force-pushed
the
optionalPeerDeps
branch
from
c70fe01 to
cdcfa74
Compare
Fuzzball24816
force-pushed
the
optionalPeerDeps
branch
from
cdcfa74 to
7bb6b7f
Compare
No description provided.