refactor: package optimizations #69
Conversation
|
Warning Rate limit exceeded@DaveMBush has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces significant changes to the ESLint configuration and utility functions in the Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant ESLint as ESLint Plugin
participant TypeChecker as Type Checking Utilities
Dev->>ESLint: Configure with new flat config
ESLint->>TypeChecker: Use new utility functions
TypeChecker-->>ESLint: Provide type analysis
ESLint-->>Dev: Lint code with enhanced type checking
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit 476b211.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-first.ts (1)
Update required: getTypeServices.ts needs migration from tsutils-etc to ts-api-utils
The verification confirms that
getTypeServices.tsdirectly depends on tsutils-etc and needs to be updated as part of the migration to ts-api-utils. This file is a critical utility that provides type checking services and uses multiple tsutils-etc functions:
couldBeTypecouldBeFunctionisAnyisReferenceTypeisUnknown🔗 Analysis chain
Line range hint
2-2: Verify if eslint-etc uses updated dependencies.Since the PR objectives mention updating from tsutils-etc to ts-api-utils, let's verify the implementation of these utility functions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of getTypeServices in eslint-etc # to ensure it's not using deprecated dependencies # Check the content of eslint-etc rg -p "getTypeServices" "../eslint-etc" # Check for any remaining uses of tsutils-etc rg -l "tsutils-etc"Length of output: 361
Script:
#!/bin/bash # Check the implementation of getTypeServices cat packages/eslint-plugin-rxjs/src/lib/eslint-etc/get-type-services.ts # Check for any imports of tsutils-etc in this file rg "tsutils-etc" packages/eslint-plugin-rxjs/src/lib/eslint-etc/get-type-services.tsLength of output: 4556
🧹 Nitpick comments (8)
packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-implement.function.ts (1)
31-53: Ensure consistent naming to reduce confusion.This function's
qualifiedparameter includes both anameand atypeChecker, while itsnodeparameter also references atypeCheckerinside the function. Renaming either could improve clarity. For instance, renamequalifiedtoqualifiedTypeCheckeror rename thetypevariable in lines 41-42 to avoid overshadowing the outer scope'stype. This helps readers quickly understand the difference between thets.Typebeing tested and thetypeCheckerused to retrieve additional information.-function isMatchingNode( - node: ts.ExpressionWithTypeArguments, - name: string | RegExp, - qualified?: { - name: RegExp; - typeChecker: ts.TypeChecker; - }, -): boolean { - const { expression } = node; - if (qualified) { - const type = qualified.typeChecker.getTypeAtLocation(expression); +function isMatchingNode( + node: ts.ExpressionWithTypeArguments, + name: string | RegExp, + qualifiedTypeChecker?: { + name: RegExp; + typeChecker: ts.TypeChecker; + }, +): boolean { + const { expression } = node; + if (qualifiedTypeChecker) { + const innerType = qualifiedTypeChecker.typeChecker.getTypeAtLocation(expression); ... }packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-reference-type.function.ts (1)
4-9: Consider adding a type guard before the cast.The type cast to
ts.ObjectTypecould be unsafe if the type is null/undefined.Consider this safer implementation:
export function isReferenceType(type: ts.Type): type is ts.TypeReference { return ( tsutils.isTypeFlagSet(type, ts.TypeFlags.Object) && - tsutils.isObjectFlagSet(type as ts.ObjectType, ts.ObjectFlags.Reference) + ((type as ts.ObjectType)?.flags !== undefined) && + tsutils.isObjectFlagSet(type as ts.ObjectType, ts.ObjectFlags.Reference) ); }packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-function.function.ts (1)
4-11: Consider optimizing the checks and handling edge cases.The implementation could benefit from:
- Adding null/undefined checks
- Optimizing the order of checks (put the fastest checks first)
- Considering additional function-like types (e.g., Method, Constructor)
Consider this enhanced implementation:
export function couldBeFunction(type: ts.Type): boolean { + if (!type) return false; return ( - type.getCallSignatures().length > 0 || couldBeType(type, 'Function') || couldBeType(type, 'ArrowFunction') || - couldBeType(type, ts.InternalSymbolName.Function) + couldBeType(type, ts.InternalSymbolName.Function) || + couldBeType(type, 'Method') || + couldBeType(type, 'Constructor') || + type.getCallSignatures().length > 0 ); }Note: The
getCallSignatures()check is moved to the end as it's likely more expensive than string comparisons.packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-type.function.ts (1)
28-31: Use optional chaining for better null safety.The baseTypes check could be simplified using optional chaining.
- const baseTypes = type.getBaseTypes(); - if (baseTypes && baseTypes.some((t) => couldBeType(t, name, qualified))) { + if (type.getBaseTypes()?.some((t) => couldBeType(t, name, qualified))) { return true; }🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
eslint.config.cjs (2)
38-43: Remove unnecessary empty rule objects.The TypeScript and JavaScript configurations spread empty rule objects which can be simplified.
.map((config) => ({ ...config, files: ['**/*.ts', '**/*.tsx', '**/*.cts', '**/*.mts'], - rules: { - ...config.rules, - }, })),Also applies to: 48-54
18-30: Consider adding more specific module boundary constraints.The current module boundary constraints are very permissive with
'*'tags. Consider adding more specific constraints based on your project's architecture.Example structure:
depConstraints: [ { - sourceTag: '*', - onlyDependOnLibsWithTags: ['*'], + sourceTag: 'rules', + onlyDependOnLibsWithTags: ['utils', 'shared'], }, ],packages/eslint-plugin-rxjs/src/lib/eslint-etc/get-type-services.ts (2)
10-34: Add JSDoc comments to TypeServices interface.The new TypeServices interface would benefit from documentation explaining the purpose of each method.
Example for the first few methods:
+/** + * Services for TypeScript type checking and analysis. + */ interface TypeServices { + /** Checks if the node could be a BehaviorSubject instance */ couldBeBehaviorSubject: (node: es.Node) => boolean; + /** Checks if the node could be an Error instance */ couldBeError: (node: es.Node) => boolean; // ... add docs for other methods
Line range hint
46-54: Add error handling for missing TypeScript services.The getType function should handle cases where TypeScript services are not available.
const getType = (node: es.Node) => { + if (!program || !typeChecker) { + throw new Error('TypeScript services not available'); + } const tsNode = esTreeNodeToTSNodeMap.get(node); if (!tsNode) { return undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.eslintrc.json(0 hunks)eslint.config.cjs(1 hunks)nx.json(2 hunks)package.json(2 hunks)packages/eslint-plugin-rxjs/.eslintrc.json(0 hunks)packages/eslint-plugin-rxjs/eslint.config.cjs(1 hunks)packages/eslint-plugin-rxjs/package.json(1 hunks)packages/eslint-plugin-rxjs/project.json(2 hunks)packages/eslint-plugin-rxjs/src/lib/eslint-etc/from-fixture.spec.ts(0 hunks)packages/eslint-plugin-rxjs/src/lib/eslint-etc/from-fixture.ts(0 hunks)packages/eslint-plugin-rxjs/src/lib/eslint-etc/get-type-services.ts(5 hunks)packages/eslint-plugin-rxjs/src/lib/eslint-etc/index.ts(0 hunks)packages/eslint-plugin-rxjs/src/lib/rules/ban-observables.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/ban-operators.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-cyclic-action.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-catch.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-first.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-subject-next.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-switchmap.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-takeuntil.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/rules/throw-error.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-function.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-type.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-implement.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/index.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-any.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-intersection-type.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-reference-type.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-type.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-union-type.function.ts(1 hunks)packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-unknown.function.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/eslint-plugin-rxjs/src/lib/eslint-etc/index.ts
- packages/eslint-plugin-rxjs/.eslintrc.json
- .eslintrc.json
- packages/eslint-plugin-rxjs/src/lib/eslint-etc/from-fixture.spec.ts
- packages/eslint-plugin-rxjs/src/lib/eslint-etc/from-fixture.ts
✅ Files skipped from review due to trivial changes (5)
- packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-takeuntil.ts
- packages/eslint-plugin-rxjs/src/lib/rules/ban-observables.ts
- packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-catch.ts
- packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-switchmap.ts
- packages/eslint-plugin-rxjs/src/lib/rules/ban-operators.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-type.function.ts
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (21)
packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-first.ts (1)
29-29: LGTM! Clean removal of stripIndent dependency.The schema description maintains clarity while successfully removing the common-tags dependency, aligning with the PR's optimization objectives.
packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-implement.function.ts (1)
3-29: Consider handling interfaces and additional edge cases.Currently, the
couldImplementfunction handles the scenario where thevalueDeclarationis specifically aClassDeclarationwith anImplementsKeyword. If your intention is to capture instances where a type could correspond to an interface or abstract class, you may need to account for those possibilities here. Additionally, types that are unions or intersections won't be handled in the present logic. Depending on your usage, you might need to add checks forInterfaceDeclarations or recursively traverse union and intersection constituents.packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-any.function.ts (1)
4-6: Looks good; straightforward implementation.This is a clean, concise helper function that leverages
ts-api-utilsfor type checks. No further changes needed.packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-unknown.function.ts (1)
4-6: Concise and correct approach.Similar to
isAny, this helper function correctly identifies unknown types by checking theTypeFlags.Unknownflag. No changes required.packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-union-type.function.ts (1)
4-6: Well-implemented type guard!Good use of type predicate (
type is ts.UnionType) for proper type narrowing in TypeScript.packages/eslint-plugin-rxjs/src/lib/tsutils-etc/index.ts (2)
1-5: Well-documented source attribution and testing status.Good practice to document the source of copied code and acknowledge the testing situation. This helps future maintainers understand the context.
6-11: Verify the copied utility functions.Since these functions were copied from tsutils-etc, we should verify their functionality in the new context.
✅ Verification successful
Utility functions are properly implemented and integrated ✓
All required utility functions are present in the codebase with proper TypeScript type signatures and are actively used in the rules implementation. The functions maintain consistency with TypeScript utility patterns and show proper integration with the existing codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the copied utility functions exist and match their source. # Check if all the exported functions exist for func in could-be-type could-be-function is-any is-reference-type is-unknown is-union-type; do if ! fd -e ts "$func.function" src/lib/tsutils-etc/; then echo "Missing function file: $func" exit 1 fi done # Compare with original source echo "Checking original source at https://github.com/cartant/tsutils-etc"Length of output: 350
Script:
#!/bin/bash # First, find the correct location of these files echo "Searching for the utility function files:" fd -e ts -t f "function\.ts$" --full-path echo -e "\nChecking file contents for tsutils-etc references:" rg -l "tsutils-etc" -g "*.ts" echo -e "\nExamining imports and exports:" rg "export.*function" -g "*.function.ts"Length of output: 2446
packages/eslint-plugin-rxjs/src/lib/tsutils-etc/is-type.function.ts (1)
3-25: Clean and well-structured type checking implementation.The function provides comprehensive type checking with:
- Proper null checks for type.symbol
- Support for both string and RegExp matching
- Optional qualified name checking with TypeScript's type checker
packages/eslint-plugin-rxjs/src/lib/tsutils-etc/could-be-type.function.ts (1)
8-37: Comprehensive type checking with good separation of concerns.The function effectively handles various type scenarios:
- Reference types
- Intersection and union types
- Base types
- Implementation checks
🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/eslint-plugin-rxjs/eslint.config.cjs (2)
37-55: Well-structured dependency checking configuration.Good configuration of @nx/dependency-checks with:
- Explicit ignored files and dependencies
- Comprehensive checks for missing, obsolete, and version mismatches
- Clear control over transitive dependencies
16-24: Consider adding specific rules for TypeScript/JavaScript files.The rules object is currently empty. Consider adding specific rules that were present in the previous configuration.
eslint.config.cjs (1)
1-8: LGTM! Good use of FlatCompat for ESLint configuration.The setup correctly uses the new flat configuration format with proper initialization of
FlatCompat.packages/eslint-plugin-rxjs/src/lib/rules/no-unsafe-subject-next.ts (1)
8-9: Verify imported functions exist in new locations.The change from
tsutilstots-api-utilsand localtsutils-etcaligns with PR objectives. Let's verify these imports:packages/eslint-plugin-rxjs/package.json (1)
5-6: Verify compatibility of updated dependencies.The update to
@typescript-eslint/utilsand addition ofts-api-utilsneeds verification:✅ Verification successful
Dependencies are compatible with project requirements
- eslint ~9.18.0 satisfies the requirement ^8.57.0 || ^9.0.0
- typescript 5.6.3 satisfies both >=4.8.4 (ts-api-utils) and >=4.8.4 <5.8.0 (@typescript-eslint/utils)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for compatibility issues between dependencies echo "Checking @typescript-eslint/utils compatibility..." npm info "@typescript-eslint/utils@^8.21.0" peerDependencies echo "Checking ts-api-utils compatibility..." npm info "ts-api-utils@^2.0.0" peerDependenciesLength of output: 398
Script:
#!/bin/bash # Check project's eslint and typescript versions echo "Project dependencies:" jq '.dependencies.eslint, .dependencies.typescript, .devDependencies.eslint, .devDependencies.typescript' packages/eslint-plugin-rxjs/package.jsonLength of output: 221
Script:
#!/bin/bash # Check root package.json for dependencies echo "Root package.json dependencies:" if [ -f "package.json" ]; then jq '.dependencies.eslint, .dependencies.typescript, .devDependencies.eslint, .devDependencies.typescript' package.json else echo "No root package.json found" fi # Look for workspace configuration echo -e "\nChecking for workspace configuration:" if [ -f "package.json" ]; then jq '.workspaces' package.json fiLength of output: 399
nx.json (1)
13-14: LGTM! Configuration updates align with ESLint flat config migration.The changes appropriately exclude the new ESLint flat config from production builds while including it in lint targets.
Also applies to: 30-31
package.json (2)
13-15: LGTM! Dependencies updated according to PR objectives.Successfully removed common-tags, tsutils, and tsutils-etc while adding ts-api-utils as a replacement.
14-14: Verify ts-api-utils compatibility with the codebase.Let's verify that the ts-api-utils version is compatible with our TypeScript version and check for any breaking changes.
✅ Verification successful
ts-api-utils v2.0.0 is compatible with the codebase
The upgrade is safe as our TypeScript version (5.6.3) exceeds the minimum requirement (4.8.4), and the current usage patterns in the codebase are not affected by any breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check ts-api-utils compatibility with our TypeScript version echo "Checking ts-api-utils compatibility..." curl -s "https://registry.npmjs.org/ts-api-utils/2.0.0" | jq -r '.engines.typescript' # Check for any breaking changes in the changelog echo "Checking for breaking changes..." curl -s "https://raw.githubusercontent.com/JoshuaKGoldberg/ts-api-utils/main/CHANGELOG.md" | head -n 20Length of output: 1476
Script:
#!/bin/bash # Check project's TypeScript version echo "Project TypeScript version:" cat package.json | jq -r '.dependencies.typescript, .devDependencies.typescript' # Get ts-api-utils v2.0.0 release notes echo -e "\nts-api-utils v2.0.0 release notes:" curl -s "https://api.github.com/repos/JoshuaKGoldberg/ts-api-utils/releases/tags/v2.0.0" # Find usage patterns echo -e "\nChecking ts-api-utils usage patterns:" rg "from ['|\"]ts-api-utils" -A 2Length of output: 6546
packages/eslint-plugin-rxjs/project.json (2)
30-30: LGTM! Correctly configured to stop bundling third-party libraries.Setting
thirdPartyto false aligns with the PR objective to no longer bundle third-party libraries.
6-16: LGTM! Release configuration properly structured.The release configuration is well-structured with appropriate package root and version resolver settings.
packages/eslint-plugin-rxjs/src/lib/rules/throw-error.ts (1)
4-4: LGTM! Import updated to use local implementation.Successfully migrated from tsutils-etc to local utility implementation.
✅ Verification successful
✅ Verified: All required utility functions exist in local implementation
The import statement correctly references the local implementations of
couldBeFunction,couldBeType,isAny, andisUnknownfunctions, which are properly exported from thetsutils-etcdirectory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the local implementation exists and matches the required interface echo "Checking local implementation..." # Look for the utility functions in the local tsutils-etc file ast-grep --pattern 'export function couldBeFunction' ast-grep --pattern 'export function couldBeType' ast-grep --pattern 'export function isAny' ast-grep --pattern 'export function isUnknown'Length of output: 274
Script:
#!/bin/bash # First, find the tsutils-etc file echo "Locating tsutils-etc file..." fd tsutils-etc.ts # If found, examine its contents echo -e "\nChecking file contents..." rg "export.*(?:couldBeFunction|couldBeType|isAny|isUnknown)" --type tsLength of output: 703
packages/eslint-plugin-rxjs/src/lib/rules/no-cyclic-action.ts (1)
35-35: LGTM! Description formatting updated.Successfully removed common-tags dependency by converting the description to a single-line string without changing the content.
| export function isIntersectionType(type: ts.Type): type is ts.UnionType { | ||
| return tsutils.isTypeFlagSet(type, ts.TypeFlags.Intersection); | ||
| } |
There was a problem hiding this comment.
Fix incorrect type predicate return type.
The function checks for TypeFlags.Intersection but incorrectly declares the return type as type is ts.UnionType. This is a type safety issue.
Apply this fix:
-export function isIntersectionType(type: ts.Type): type is ts.UnionType {
+export function isIntersectionType(type: ts.Type): type is ts.IntersectionType {
return tsutils.isTypeFlagSet(type, ts.TypeFlags.Intersection);
}📝 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.
| export function isIntersectionType(type: ts.Type): type is ts.UnionType { | |
| return tsutils.isTypeFlagSet(type, ts.TypeFlags.Intersection); | |
| } | |
| export function isIntersectionType(type: ts.Type): type is ts.IntersectionType { | |
| return tsutils.isTypeFlagSet(type, ts.TypeFlags.Intersection); | |
| } |
| "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" | ||
| }, |
There was a problem hiding this comment.
🛠️ 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.
| "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" | |
| } |
There was a problem hiding this comment.
There are reasons to leave this out as not every rule requires typescript
There was a problem hiding this comment.
@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.
Issue Number: #66 #67 #68 #52
Body
Update to eslint flat config for project
Remove dependency on common-tags
Remove dependency on tsutils-etc
Build no longer bundles 3rd party libs
Update tsutils to ts-api-utils
Remove unused files
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
Package Updates
Configuration Changes
.eslintrc.jsonto neweslint.config.cjs.Code Maintenance
Type Utilities