-
Notifications
You must be signed in to change notification settings - Fork 0
Fixed type-only import for bare-bone pattern #140
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
Conversation
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.
Pull request overview
This PR fixes the handling of type-only imports in the require-aws-bare-bones ESLint rule. The rule now correctly ignores declaration-level type imports (import type { ... }) from AWS SDK packages.
Key changes:
- Added check for declaration-level type imports (
node.importKind === 'type') - Refactored suffix arrays from
Set<string>tostring[]for simplicity - Removed per-specifier type import check
- Added test case for declaration-level type imports
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/aws/require-aws-bare-bones.ts | Added declaration-level type import check and simplified suffix data structures |
| src/aws/require-aws-bare-bones.spec.ts | Added test case for import type syntax |
| package.json | Version bump from 7.17.0 to 7.17.1 |
| package-lock.json | Lockfile version update to match package.json |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| { | ||
| code: `import type { Credentials } from '@aws-sdk/client-sts';`, | ||
| }, |
Copilot
AI
Nov 24, 2025
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.
The only: true flag should be removed. This flag is typically used during development to run only a specific test, but should not be committed to the codebase as it will cause all other tests to be skipped.
|
Beta Published - Install Command: |
|
Tested in local services |
|
❌ PR review status - has 1 reviewer outstanding |
le-cong
left a comment
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.
lftm
Closes #139