search

import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.58k stars 1.57k forks source link

Add ignore pattern before using unusedExports by default #2970

Open Florian-crg opened 9 months ago

Florian-crg commented 9 months ago

Before doing https://github.com/import-js/eslint-plugin-import/issues/1324 We should have a varsIgnorePattern or similar. There is cases where you don't want to remove all unsued exports. For example in some project when we have a custom library / DS we export the props from the component files. Though we do not want to ignore the files itself, only interface / type that end with Props

ljharb commented 9 months ago

This seems like it would promote the antipattern of "barrel files" - the best practice is for each file to export a single thing. This is ofc less important for types, but I'm not clear on why you'd want to check the JS exports but not the types?

Florian-crg commented 9 months ago

This seems like it would promote the antipattern of "barrel files" - the best practice is for each file to export a single thing. This is ofc less important for types, but I'm not clear on why you'd want to check the JS exports but not the types?

Thanks for raising the potential antipattern of "barrel files". Which I agree we should avoid. In our case it is not about "barrel files", the goal is not to check the JS exports but not the types.

The use cas would apply for example when we got a custom built-in library. This library has components. A component .tsx in our library has two exports. One default export for the component one for the props.

We don't want developers to edit the DS to export the props when other developers need it. Similar to the flexibility offered by no-unused-vars with varIgnorePattern, my suggestion aims to provide some control over unused export checks.

ljharb commented 9 months ago

In that case, would perhaps an ignoreTypes boolean option suffice?

Florian-crg commented 9 months ago

The problem of the ignoreTypes boolean is that it would disable checks for all types, which is much more destructive, leading to more unseen dead code. Which is what we want to avoid. By targeting using a similar pattern of the no-unused-vars we keep control of what we want to be dead code or not.

ljharb commented 9 months ago

I'm still very confused.

I understand why you'd want to check for unused exported types. I also understand why you'd want to have entry point files (whether they export types or values or both) not checked.

I'm not clear on why, within a single file, you'd want some items checked and some not, since if a file is an entry point, everything it exports is part of its API.

Florian-crg commented 9 months ago

Got it! Let me clarify with an example:

Consider MyBox.tsx:

export interface MyBoxProps {
  // interface definition
}
...
export default MyBox;

In this case, MyBox.tsx serves as a component file within our library. We export both the component itself and its associated props. While we want to ensure that the component is always exported and used where necessary, we don't necessarily need to enforce the same requirement for its props interface.

By allowing selective checking, we strike a balance between ensuring that their is no used export / dead code while still providing flexibility for developers to use supporting elements as needed.

Let me know if this clarifies things!

ljharb commented 9 months ago

ok, so this isn't for entrypoints at all, it's because you want every component to have the same exported types even if they're unused?

what happens if the component is unused but the types are used?

Florian-crg commented 9 months ago

Not sure to follow what you mean by "ok, so this isn't for entrypoints at all, it's because you want every component to have the same exported types even if they're unused?".

Each component its prop. MyBoxProp for MyBox or MyOtherProps for another component.

what happens if the component is unused but the types are used?

I'm not sure for which context your question is. But let's say we have a kinda exportIgnorePattern which says \w+Props\b the type is used (or not eslint) does not yell.

The component is unused, eslint yell.

If you're afraid of users deleting the component while the type is used, I'd say, if the user has added an ignore pattern then they should be carful.