-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Enhancement: Allow specifying properties in TypeOrValueSpecifier #10740
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
Comments
👍 this makes sense to me as a feature request. The difficult point here is that we don't yet have a way in the shared TypeOrValueSpecifier format to specify a property of something imported by a package. Copying the current interface from https://typescript-eslint.io/packages/type-utils/type-or-value-specifier#packagespecifier: interface PackageSpecifier {
from: 'package';
name: string[] | string;
package: string;
} How do we think this could be augmented to allow both { "from": "package", "name": "it", "package": "node:test", "properties": ["skip", "only"] } { "from": "package", "name": ["it", { "object": "it", "properties": ["skip", "only"] }], "package": "vitest" } |
Could we more or less use the actual JS syntax as the specifier syntax? { "from": "package", "package": "node:test", "name": "it" },
{ "from": "package", "package": "node:test", "name": "it.only" },
{ "from": "package", "package": "node:test", "name": "it.skip" }, PS - how long until we start accepting arbitrary ESquery syntax 🙃 { "from": "package", "package": "node:test", "selector": "Identifier[name=it]" },
{ "from": "package", "package": "node:test", "selector": "MemberExpression[property=only][object.type=Identifier][object.name=it]" },
{ "from": "package", "package": "node:test", "selector": "MemberExpression[property=skip][object.type=Identifier][object.name=it]" }, |
How would this extend to multiple properties? Like
😱 I never thought of using ESQuery syntax, that's a really interesting idea... |
Good point..... I mean, the other choice, which at first sounds like it wouldn't be crazy, would be regex. /(describe|it|test)(\.(only|skip|runIf)?/ But this obviously fails quickly for things like describe
.it(
'foo',
() => {}
); I guess the question is would we want to be very general at the expense of being very unwieldy (direction of ESQuery), or very straightforward at the expense of being very constrained (direction of the syntaxes you've described). Trying to hit something in between seems like we'll just do a bad job at a complex problem (like the regex idea). |
See also @Harpush's #11083 - another use case for this feature would be specifying specific properties in
Coming back to this after a few months, I think it's fine to take the "straightforward at the expense of being very constrained" approach. Properties are kind of a special case for My current proposal: let's go with My only concern here is that feels a little ambiguous. Users might look at it and wonder if |
How about: { "from": "lib", "allowedName": "KeydownEvent" } and { "from": "lib", "name": "KeydownEvent", "allowedProperties": [ "keyCode" ] } It changes quite a bit the type semantics but it is very straightforward.... Another option is just adding a note about it in docs that properties override the name (switching) and to allow both you need to specify both |
Before You File a Proposal Please Confirm You Have Done The Following...
My proposal is suitable for this project
Link to the rule's documentation
https://typescript-eslint.io/rules/no-floating-promises/#allowforknownsafepromises
Description
Hi there,
@typescript-eslint/no-floating-promises
's allowForKnownSafePromiseReturns #8404 works well for it fromnode:test
. However, it doesn't help forit.only
andit.skip
. They are handy during debugging so it would be great to have the ability to ignore them as well.Fail
Pass
Additional Info
No response
The text was updated successfully, but these errors were encountered: