Skip to content
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.

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

refactor: package optimizations #69

Merged
merged 10 commits into from
Jan 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 0 additions & 42 deletions .eslintrc.json

This file was deleted.

68 changes: 68 additions & 0 deletions eslint.config.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
const { FlatCompat } = require('@eslint/eslintrc');
const js = require('@eslint/js');
const nxEslintPlugin = require('@nx/eslint-plugin');

const compat = new FlatCompat({
baseDirectory: __dirname,
recommendedConfig: js.configs.recommended,
});

module.exports = [
{
ignores: ['**/dist'],
},
{ plugins: { '@nx': nxEslintPlugin } },
{
files: ['**/*.ts', '**/*.tsx', '**/*.js', '**/*.jsx'],
rules: {
'@nx/enforce-module-boundaries': [
'error',
{
enforceBuildableLibDependency: true,
allow: [],
depConstraints: [
{
sourceTag: '*',
onlyDependOnLibsWithTags: ['*'],
},
],
},
],
},
},
...compat
.config({
extends: ['plugin:@nx/typescript'],
})
.map((config) => ({
...config,
files: ['**/*.ts', '**/*.tsx', '**/*.cts', '**/*.mts'],
rules: {
...config.rules,
},
})),
...compat
.config({
extends: ['plugin:@nx/javascript'],
})
.map((config) => ({
...config,
files: ['**/*.js', '**/*.jsx', '**/*.cjs', '**/*.mjs'],
rules: {
...config.rules,
},
})),
...compat
.config({
env: {
jest: true,
},
})
.map((config) => ({
...config,
files: ['**/*.spec.ts', '**/*.spec.tsx', '**/*.spec.js', '**/*.spec.jsx'],
rules: {
...config.rules,
},
})),
];
6 changes: 4 additions & 2 deletions nx.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"!{projectRoot}/tsconfig.spec.json",
"!{projectRoot}/jest.config.[jt]s",
"!{projectRoot}/src/test-setup.[jt]s",
"!{projectRoot}/test-setup.[jt]s"
"!{projectRoot}/test-setup.[jt]s",
"!{projectRoot}/eslint.config.cjs"
],
"sharedGlobals": []
},
Expand All @@ -26,7 +27,8 @@
"default",
"{workspaceRoot}/.eslintrc.json",
"{workspaceRoot}/.eslintignore",
"{workspaceRoot}/eslint.config.js"
"{workspaceRoot}/eslint.config.js",
"{workspaceRoot}/eslint.config.cjs"
]
},
"@nx/jest:jest": {
Expand Down
13 changes: 5 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
},
"private": true,
"dependencies": {
"@typescript-eslint/utils": "^8.20.0",
"common-tags": "^1.8.2",
"tslib": "^2.8.1",
"tsutils": "^3.21.0",
"tsutils-etc": "^1.4.2"
"@typescript-eslint/utils": "^8.21.0",
"ts-api-utils": "^2.0.0",
"tslib": "^2.8.1"
},
"devDependencies": {
"@angular-eslint/test-utils": "19.0.2",
"@eslint/eslintrc": "^2.1.1",
"@nx/angular": "20.3.2",
"@nx/cypress": "20.3.2",
"@nx/esbuild": "20.3.2",
Expand All @@ -33,11 +32,8 @@
"@swc-node/register": "~1.10.9",
"@swc/core": "~1.10.7",
"@swc/helpers": "~0.5.15",
"@types/common-tags": "^1.8.0",
"@types/jest": "^29.4.0",
"@types/node": "22.10.5",
"@typescript-eslint/eslint-plugin": "^8.20.0",
"@typescript-eslint/parser": "^8.20.0",
"@typescript-eslint/rule-tester": "^8.20.0",
"esbuild": "^0.24.2",
"eslint": "~9.18.0",
Expand All @@ -50,6 +46,7 @@
"ts-jest": "^29.2.5",
"ts-node": "10.9.2",
"typescript": "5.6.3",
"typescript-eslint": "^8.13.0",
"verdaccio": "^6.0.5"
},
"engines": {
Expand Down
33 changes: 0 additions & 33 deletions packages/eslint-plugin-rxjs/.eslintrc.json

This file was deleted.

61 changes: 61 additions & 0 deletions packages/eslint-plugin-rxjs/eslint.config.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
const { FlatCompat } = require('@eslint/eslintrc');
const js = require('@eslint/js');
const baseConfig = require('../../eslint.config.cjs');

const compat = new FlatCompat({
baseDirectory: __dirname,
recommendedConfig: js.configs.recommended,
});

module.exports = [
{
ignores: ['**/dist'],
},
...baseConfig,
{
files: ['**/*.ts', '**/*.tsx', '**/*.js', '**/*.jsx'],
// Override or add rules here
rules: {},
languageOptions: {
parserOptions: {
project: ['packages/eslint-plugin-rxjs/tsconfig.*?.json'],
},
},
},
{
files: ['**/*.ts', '**/*.tsx'],
// Override or add rules here
rules: {},
},
{
files: ['**/*.js', '**/*.jsx'],
// Override or add rules here
rules: {},
},
{
files: ['**/*.json'],
rules: {
'@nx/dependency-checks': [
'error',
{
ignoredFiles: ['{projectRoot}/esbuild.config.{js,ts,mjs,mts}'],
ignoredDependencies: [
'typescript',
'@angular-eslint/test-utils',
'@eslint/eslintrc',
'@eslint/js',
'jsonc-eslint-parser',
'@typescript-eslint/rule-tester',
],
checkMissingDependencies: true,
checkObsoleteDependencies: true,
checkVersionMismatches: true,
includeTransitiveDependencies: false,
},
],
},
languageOptions: {
parser: require('jsonc-eslint-parser'),
},
},
];
10 changes: 2 additions & 8 deletions packages/eslint-plugin-rxjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,8 @@
"name": "@smarttools/eslint-plugin-rxjs",
"version": "1.0.0",
"dependencies": {
"@typescript-eslint/utils": "^8.19.0",
"@typescript-eslint/rule-tester": "^8.19.1",
"common-tags": "^1.8.2",
"decamelize": "^6.0.0",
"typescript": "~5.5.4",
"tsutils": "^3.21.0",
"tsutils-etc": "^1.4.2",
"@angular-eslint/test-utils": "19.0.2"
"@typescript-eslint/utils": "^8.21.0",
"ts-api-utils": "^2.0.0"
},
Comment on lines 4 to 7
Copy link

@coderabbitai coderabbitai bot Jan 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding TypeScript as a peer dependency.

Since the plugin relies heavily on TypeScript functionality, it should specify TypeScript as a peer dependency.

   "dependencies": {
     "@typescript-eslint/utils": "^8.21.0",
     "ts-api-utils": "^2.0.0"
   },
+  "peerDependencies": {
+    "typescript": ">=4.7.0"
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"dependencies": {
"@typescript-eslint/utils": "^8.19.0",
"@typescript-eslint/rule-tester": "^8.19.1",
"common-tags": "^1.8.2",
"decamelize": "^6.0.0",
"typescript": "~5.5.4",
"tsutils": "^3.21.0",
"tsutils-etc": "^1.4.2",
"@angular-eslint/test-utils": "19.0.2"
"@typescript-eslint/utils": "^8.21.0",
"ts-api-utils": "^2.0.0"
},
"dependencies": {
"@typescript-eslint/utils": "^8.21.0",
"ts-api-utils": "^2.0.0"
},
"peerDependencies": {
"typescript": ">=4.7.0"
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

There are reasons to leave this out as not every rule requires typescript

Copy link

Choose a reason for hiding this comment

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

@DaveMBush I see your point. After analyzing the codebase, indeed most rules (about 90%) don't directly depend on TypeScript functionality. They appear to be pure ESLint rules that work with JavaScript syntax. I apologize for the incorrect suggestion.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find all rule files
echo "=== Rule files ==="
fd "\.ts$" "packages/eslint-plugin-rxjs/src/lib/rules"

# Check TypeScript imports in rule files
echo -e "\n=== Files importing TypeScript (ts) ==="
rg "import.*ts['\"]" "packages/eslint-plugin-rxjs/src/lib/rules"

# Check files not using TypeScript
echo -e "\n=== Files not importing TypeScript ==="
fd "\.ts$" "packages/eslint-plugin-rxjs/src/lib/rules" --exec sh -c 'if ! grep -q "import.*ts" "$1"; then echo "$1"; fi' sh {}

Length of output: 5474


✏️ Learnings added
Learnt from: DaveMBush
PR: DaveMBush/eslint-plugin-rxjs#69
File: packages/eslint-plugin-rxjs/package.json:4-7
Timestamp: 2025-01-25T19:46:28.370Z
Learning: In eslint-plugin-rxjs, TypeScript should not be added as a peer dependency since most rules don't require TypeScript functionality. Only specific rules like no-unsafe-switchmap, no-unsafe-first, no-unsafe-catch, and no-cyclic-action use TypeScript features.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

"repository": {
"type": "git",
Expand Down
51 changes: 20 additions & 31 deletions packages/eslint-plugin-rxjs/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,36 @@
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "packages/eslint-plugin-rxjs/src",
"projectType": "library",
"release": {
"version": {
"generatorOptions": {
"packageRoot": "dist/{projectRoot}",
"currentVersionResolver": "git-tag"
}
}
},
"nxDependencyChecks": {
"ignoredDependencies": ["typescript", "@angular-eslint/test-utils"]
},
"tags": [],
"targets": {
"build-lib": {
"executor": "@nx/esbuild:esbuild",
"outputs": [
"{options.outputPath}"
],
"outputs": ["{options.outputPath}"],
"options": {
"outputPath": "dist/packages/eslint-plugin-rxjs",
"main": "packages/eslint-plugin-rxjs/src/index.ts",
"tsConfig": "packages/eslint-plugin-rxjs/tsconfig.lib.json",
"assets": [
"packages/eslint-plugin-rxjs/*.md"
],
"format": [
"cjs"
],
"assets": ["packages/eslint-plugin-rxjs/*.md"],
"format": ["cjs"],
"dts": true,
"bundle": true,
"thirdParty": true
"thirdParty": false
}
},
"build": {
"executor": "nx:run-commands",
"dependsOn": [
"build-lib"
],
"dependsOn": ["build-lib"],
"options": {
"command": "echo 'Copying types...' && cp dist/packages/eslint-plugin-rxjs/src/index.d.ts dist/packages/eslint-plugin-rxjs"
}
Expand All @@ -40,32 +44,17 @@
},
"lint": {
"executor": "@nx/eslint:lint",
"outputs": [
"{options.outputFile}"
],
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": [
"packages/eslint-plugin-rxjs/package.json"
]
"lintFilePatterns": ["packages/eslint-plugin-rxjs/package.json"]
}
},
"test": {
"executor": "@nx/jest:jest",
"outputs": [
"{workspaceRoot}/coverage/{projectRoot}"
],
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "packages/eslint-plugin-rxjs/jest.config.ts"
}
}
},
"tags": [],
"release": {
"version": {
"generatorOptions": {
"packageRoot": "dist/{projectRoot}",
"currentVersionResolver": "git-tag"
}
}
}
}
Loading
Loading