-
Notifications
You must be signed in to change notification settings - Fork 1
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
Dmb/circular dependency error when using recommended/49 #59
Changes from all commits
8a44e29
0d2f9ea
816dfb2
553b58b
e02f5ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Tests | ||
|
||
The tests in this directory tests all the ways that eslint can be used in combination with this plugin. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
const util = require('util'); | ||
const rxjs = require('@smarttools/eslint-plugin-rxjs'); | ||
|
||
const result =util.inspect(rxjs, { depth: null }); | ||
|
||
if (result.includes('[Circular')) { | ||
console.log('Circular reference found'); | ||
console.log(result); | ||
process.exit(1); | ||
} else { | ||
console.log('No circular reference found'); | ||
process.exit(0); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
#!/bin/bash | ||
|
||
# Exit on any error | ||
set -e | ||
|
||
# Get the directory where the script is located | ||
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | ||
ROOT_DIR="$SCRIPT_DIR/../.." | ||
|
||
# Define source and destination paths relative to root | ||
SRC_DIR="$ROOT_DIR/dist/packages/eslint-plugin-rxjs" | ||
DEST_DIR="$ROOT_DIR/node_modules/@smarttools/eslint-plugin-rxjs" | ||
|
||
# Check if source directory exists | ||
if [ ! -d "$SRC_DIR" ]; then | ||
echo "Error: Source directory $SRC_DIR does not exist" | ||
exit 1 | ||
fi | ||
|
||
# Create destination parent directory if it doesn't exist | ||
mkdir -p "$(dirname "$DEST_DIR")" | ||
|
||
# Remove destination directory if it exists | ||
if [ -d "$DEST_DIR" ]; then | ||
rm -rf "$DEST_DIR" | ||
fi | ||
|
||
# Copy the directory | ||
if cp -r "$SRC_DIR" "$DEST_DIR"; then | ||
echo "✓ Successfully copied $SRC_DIR to $DEST_DIR" | ||
else | ||
echo "✗ Failed to copy directory" | ||
exit 1 | ||
fi | ||
|
||
# Run the index.js file | ||
echo "Running index.js..." | ||
if node "$SCRIPT_DIR/index.js"; then | ||
echo "✓ Tests completed successfully" | ||
exit 0 | ||
else | ||
echo "✗ Tests failed" | ||
exit 1 | ||
fi | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"extends": "../../tsconfig.base.json" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
module.exports = { | ||
extends: ['plugin:@smarttools/rxjs/recommended-legacy'], | ||
rules: { | ||
'@smarttools/rxjs/finnish': [ | ||
'warn', | ||
{ | ||
functions: false, | ||
methods: false, | ||
names: { | ||
'^(canActivate|canActivateChild|canDeactivate|canLoad|intercept|resolve|validate|store|_store)$': | ||
false, | ||
}, | ||
parameters: false, | ||
properties: false, | ||
strict: false, | ||
types: { | ||
'^EventEmitter$': false, | ||
}, | ||
variables: true, | ||
}, | ||
], | ||
'@smarttools/rxjs/suffix-subjects': [ | ||
'warn', | ||
{ | ||
parameters: false, | ||
properties: false, | ||
suffix: '$$', | ||
variables: true, | ||
}, | ||
], | ||
'@smarttools/rxjs/no-nested-subscribe': ['warn'], | ||
'@smarttools/rxjs/no-implicit-any-catch': ['off'], | ||
'@smarttools/rxjs/no-unsafe-takeuntil': [ | ||
'warn', | ||
{ | ||
alias: ['untilDestroyed'], | ||
}, | ||
], | ||
'@smarttools/rxjs/no-async-subscribe': 'warn', | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
const { FlatCompat } = require('@eslint/eslintrc'); | ||
const js = require('@eslint/js'); | ||
const join = require('path').join; | ||
const rxjs = require('@smarttools/eslint-plugin-rxjs'); | ||
|
||
const compat = new FlatCompat({ | ||
baseDirectory: __dirname, | ||
recommendedConfig: js.configs.recommended, | ||
}); | ||
|
||
module.exports = [ | ||
{ | ||
ignores: ['**/dist'], | ||
}, | ||
{ | ||
plugins: { | ||
'@smarttools/rxjs': rxjs, | ||
}, | ||
}, | ||
...compat | ||
.config({ | ||
extends: ['./base.eslint.config.js'], | ||
}) | ||
.map((config) => ({ | ||
...config, | ||
files: ['**/*.ts'], | ||
languageOptions: { | ||
parser: require('@typescript-eslint/parser'), | ||
parserOptions: { | ||
project: join(__dirname, './tsconfig.json'), | ||
}, | ||
}, | ||
rules: { | ||
...config.rules, | ||
}, | ||
})), | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"type": "commonjs" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// just a sample to run lint against | ||
|
||
import { from } from 'rxjs'; | ||
|
||
from([1, 2, 3]).subscribe((x) => console.log(x)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
#!/bin/bash | ||
|
||
# Exit on error | ||
set -e | ||
|
||
# Get the directory where the script is located | ||
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | ||
ROOT_DIR="$SCRIPT_DIR/../.." | ||
|
||
# Define source and destination paths relative to root | ||
SRC_DIR="$ROOT_DIR/dist/packages/eslint-plugin-rxjs" | ||
DEST_DIR="$ROOT_DIR/node_modules/@smarttools/eslint-plugin-rxjs" | ||
|
||
# Check if source directory exists | ||
if [ ! -d "$SRC_DIR" ]; then | ||
echo "Error: Source directory $SRC_DIR does not exist" | ||
exit 1 | ||
fi | ||
|
||
# Create destination parent directory if it doesn't exist | ||
mkdir -p "$(dirname "$DEST_DIR")" | ||
|
||
# Remove destination directory if it exists | ||
if [ -d "$DEST_DIR" ]; then | ||
rm -rf "$DEST_DIR" | ||
fi | ||
|
||
# Copy the directory | ||
if cp -r "$SRC_DIR" "$DEST_DIR"; then | ||
echo "✓ Successfully copied $SRC_DIR to $DEST_DIR" | ||
else | ||
echo "✗ Failed to copy directory" | ||
exit 1 | ||
fi | ||
|
||
# Run ESLint with the specific config and test file | ||
if pnpm eslint --config $SCRIPT_DIR/eslint.config.js $SCRIPT_DIR/sample.ts; then | ||
echo -e "\n✅ Linting PASSED" | ||
else | ||
echo -e "\n❌ Linting FAILED" | ||
exit 1 | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"extends": "../../tsconfig.base.json" | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,53 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#!/bin/bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Exit on error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
set -e | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Get the directory where the script is located | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Initialize counters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TOTAL_TESTS=4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PASSED_TESTS=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FAILED_TESTS=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FAILED_NAMES=() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Function to run a test and track its result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function run_test() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
local test_name=$1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
local test_path=$2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo -e "\n📋 Running test: $test_name" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if bash "$test_path"; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "✅ $test_name passed" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PASSED_TESTS=$((PASSED_TESTS + 1)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "❌ $test_name failed" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FAILED_TESTS=$((FAILED_TESTS + 1)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FAILED_NAMES+=("$test_name") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+15
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance test runner with timeout and error handling. The current implementation could benefit from additional error handling and debugging capabilities. function run_test() {
local test_name=$1
local test_path=$2
+ local timeout_seconds=300 # 5 minutes timeout
+ local output_file
+ output_file=$(mktemp)
echo -e "\n📋 Running test: $test_name"
- if bash "$test_path"; then
+ if timeout "$timeout_seconds" bash "$test_path" > "$output_file" 2>&1; then
echo "✅ $test_name passed"
PASSED_TESTS=$((PASSED_TESTS + 1))
else
- echo "❌ $test_name failed"
+ local exit_code=$?
+ echo "❌ $test_name failed with exit code $exit_code"
+ echo "Last 10 lines of output:"
+ tail -n 10 "$output_file"
FAILED_TESTS=$((FAILED_TESTS + 1))
FAILED_NAMES+=("$test_name")
fi
+ rm -f "$output_file"
} 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've created a followup task for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DaveMBush Thanks for creating a follow-up task! Let me know if you need any assistance when you get to implementing those enhancements. If you found this review helpful, would you consider giving us a shout-out on X? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Run each test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
run_test "ESM JS Tests" "$SCRIPT_DIR/esm-js/test.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
run_test "Using Recommended Tests" "$SCRIPT_DIR/using-recommended/test.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
run_test "Extends From Tests" "$SCRIPT_DIR/extends-from/test.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
run_test "Circular Reference Tests" "$SCRIPT_DIR/circular-reference/test.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Print summary report | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo -e "\n📊 Test Summary Report" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "=======================" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Total Tests: $TOTAL_TESTS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Passed: $PASSED_TESTS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Failed: $FAILED_TESTS" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if [ ${#FAILED_NAMES[@]} -ne 0 ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo -e "\n❌ Failed Tests:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for name in "${FAILED_NAMES[@]}"; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "- $name" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo -e "\n✨ All tests passed!" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
exit 0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
const rxjs = require('@smarttools/eslint-plugin-rxjs'); | ||
const typescriptEslintParser = require('@typescript-eslint/parser'); | ||
|
||
module.exports = [ | ||
rxjs.configs.recommended, | ||
{ | ||
files: ['**/*.ts'], | ||
plugins: { | ||
rxjs, | ||
}, | ||
}, | ||
{ | ||
languageOptions: { | ||
parser: typescriptEslintParser, | ||
parserOptions: { | ||
project: './tsconfig.json', | ||
tsconfigRootDir: __dirname, | ||
}, | ||
}, | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"type": "commonjs" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// just a sample to run lint against | ||
|
||
import { from } from 'rxjs'; | ||
|
||
from([1, 2, 3]).subscribe((x) => console.log(x)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
#!/bin/bash | ||
|
||
# Exit on error | ||
set -e | ||
|
||
# Get the directory where the script is located | ||
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | ||
ROOT_DIR="$SCRIPT_DIR/../.." | ||
|
||
# Define source and destination paths relative to root | ||
SRC_DIR="$ROOT_DIR/dist/packages/eslint-plugin-rxjs" | ||
DEST_DIR="$ROOT_DIR/node_modules/@smarttools/eslint-plugin-rxjs" | ||
|
||
# Check if source directory exists | ||
if [ ! -d "$SRC_DIR" ]; then | ||
echo "Error: Source directory $SRC_DIR does not exist" | ||
exit 1 | ||
fi | ||
|
||
# Create destination parent directory if it doesn't exist | ||
mkdir -p "$(dirname "$DEST_DIR")" | ||
|
||
# Remove destination directory if it exists | ||
if [ -d "$DEST_DIR" ]; then | ||
rm -rf "$DEST_DIR" | ||
fi | ||
|
||
# Copy the directory | ||
if cp -r "$SRC_DIR" "$DEST_DIR"; then | ||
echo "✓ Successfully copied $SRC_DIR to $DEST_DIR" | ||
else | ||
echo "✗ Failed to copy directory" | ||
exit 1 | ||
fi | ||
|
||
# Run ESLint with the specific config and test file | ||
if pnpm eslint --config $SCRIPT_DIR/eslint.config.js $SCRIPT_DIR/sample.ts; then | ||
echo -e "\n✅ Linting PASSED" | ||
else | ||
echo -e "\n❌ Linting FAILED" | ||
exit 1 | ||
fi | ||
Comment on lines
+1
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider creating a shared test utilities script. This script shares significant code with other test scripts. Consider extracting common functionality into a shared utilities script. Create a new file #!/bin/bash
setup_test_env() {
local root_dir="$1"
local script_dir="$2"
# Create temp directory
local temp_dir=$(mktemp -d)
local dest_dir="$temp_dir/node_modules/@smarttools/eslint-plugin-rxjs"
local src_dir="$root_dir/dist/packages/eslint-plugin-rxjs"
# Copy plugin to test location
mkdir -p "$(dirname "$dest_dir")"
cp -r "$src_dir" "$dest_dir" || {
echo "✗ Failed to copy directory"
rm -rf "$temp_dir"
return 1
}
echo "✓ Successfully copied $src_dir to $dest_dir"
echo "$temp_dir"
}
cleanup_test_env() {
local temp_dir="$1"
rm -rf "$temp_dir"
echo "Cleaned up test resources"
} Then modify this script to: #!/bin/bash
set -e
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
ROOT_DIR="$SCRIPT_DIR/../.."
# Source common utilities
source "$ROOT_DIR/tests/utils.sh"
# Setup test environment
TEMP_DIR=$(setup_test_env "$ROOT_DIR" "$SCRIPT_DIR")
trap 'cleanup_test_env "$TEMP_DIR"' EXIT
# Run ESLint with the specific config and test file
if pnpm eslint --config "$SCRIPT_DIR/eslint.config.js" "$SCRIPT_DIR/sample.ts"; then
echo -e "\n✅ Linting PASSED"
exit 0
else
echo -e "\n❌ Linting FAILED"
exit 1
fi There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've created a followup task for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DaveMBush Thanks for creating a follow-up task! Let me know if you need any assistance when you get to implementing the shared utilities. If you found this review helpful, would you consider giving us a shout-out on X? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add cleanup for temporary resources.
The script should clean up resources even if tests fail.
📝 Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created a followup task for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DaveMBush Thanks for creating a follow-up task! That's a great way to ensure the cleanup functionality gets implemented.
⟦ ∙ᴥ∙ ⟧ happy to help