-
-
Notifications
You must be signed in to change notification settings - Fork 322
detect test files if tsx --test script is present
#1090
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
|
The binary issue seems to be fixed. Is my solution correct? |
webpro
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.
Thanks Nick! Happy to merge, but I do have a remark.
| const resolveConfig: ResolveConfig<PackageJson> = localConfig => { | ||
| const scripts = localConfig.scripts; | ||
|
|
||
| const entries = [toDependency('tsx')]; |
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.
This one should probably not be added here. If tsx is listed in package.json but not used anywhere we probably want it reported as unused.
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.
fixed
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "name": "tsx", | |||
| "bin": "" | |||
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.
What I typically do is add an empty index.js file with "bin": "./index.js".
Knip looks here for the tsx executable referenced in package.json#scripts and understands the tsx dependency is used.
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.
fixed
commit: |
| import type { Plugin } from '../../types/config.js'; | ||
| import type { PackageJson } from 'src/types/package-json.js'; | ||
| import { toEntry } from 'src/util/input.js'; | ||
| import { hasDependency } from 'src/util/plugin.js'; |
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.
Please use relative imports
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.
fixed
|
Nice one! 🙏 |
|
🚀 This pull request is included in v5.57.0. See Release 5.57.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
Similar to how the node plugin detects test files if
node --testis present.Documented here: https://tsx.is/node-enhancement#test-runner
Two issues:
toBinaryincorrectly. The tests are failing.isEnabledwasn't being used for the tsx plugin before.