Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): no-empty-interface - add allowSingleExtend option#215
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
feat(eslint-plugin): no-empty-interface - add allowSingleExtend option#215
Uh oh!
There was an error while loading.Please reload this page.
Conversation
d70553b
to5cf9322
Comparecodecovbot commentedFeb 5, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #215 +/- ##==========================================+ Coverage 94.63% 94.63% +<.01%========================================== Files 66 66 Lines 2794 2796 +2 Branches 723 724 +1 ==========================================+ Hits 2644 2646 +2 Misses 57 57 Partials 93 93
|
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
5cf9322
to59bae94
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
59bae94
to9789843
CompareThere 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.
code LGTM.
Good work on your first contribution 👍
Uh oh!
There was an error while loading.Please reload this page.
Brad seems to agree with my suggested name change
758bdfb
toab970d3
CompareHey@timkraut, sorry for the pain, but we just merged a big PR to convert the plugin to typescript. |
1b57c48
to3952206
Compare@bradzacher Don't worry, it totally makes sense to use TypeScript in this case. I've converted the PR to TypeScript. Please check again |
Hi,
this PR adds an option to allow a single extend (option:
allowSingleExtend: true|false
) to the ruleno-empty-interface
. Our use case for this is the following:Use case
We are building a React application with TypeScript. In React, you typically have components which might look like this:
Problem
Let's say our
<ProductTile>
component accepts the exact same props as the members defined in aninterface Product { ... }
. We could change the definition of the<ProductTile>
component as follows to support that:But once we have to add an additional prop (e.g.
onClick?(event: MouseEvent<HTMLButtonElement>): void
), we have the issue that we would have to extendProduct
(which doesn't make sense asonClick
doesn't belong to aProduct
) or replace the interface.Solution
Instead, we would prefer to define the component as follows initially:
Now if we add an additional prop like
onClick
, we could simply add it to the interfaceProductTileProps
and we're done:This PR allows to do that while still warning if empty interfaces like
interface Foo {}
are used. It's an optional configuration property so that the default behavior is not changed.P.S.: This is my first PR to change an ESLint-style rule. Let me know if there is anything I should do differently. Thanks a lot!