eslint-plugin-deprecation
eslint-plugin-deprecation copied to clipboard
deprecation warnings from barrel
Hi, with the following setting
"deprecation/deprecation": "warn"
I get warnings from my barrel (index.ts)
Like this
~\Repos\akita\libs\akita\src\lib\index.ts
4:10 warning 'arrayAdd' is deprecated. deprecation/deprecation
But it's not using anything, it's exporting
export { arrayAdd } from './arrayAdd';
Another thing. It's warning about functions from my own source files that have been marked as @deprecated
. Perhaps that could be an option if it should do that or not.
export function arrayAdd<Root extends any[], Entity = any>(keyOrRoot: Root, newEntity: OrArray<Root[0]>, options?: AddEntitiesOptions): Root[0][];
/**
* @deprecated
*/
export function arrayAdd<Root, Entity = any>(keyOrRoot: ArrayProperties<Root>, newEntity: OrArray<Entity>, options?: AddEntitiesOptions): (state: Root) => Root;
export function arrayAdd<Root, Entity = any>(keyOrRoot: ArrayProperties<Root> | Root, newEntity: OrArray<Entity>, options: AddEntitiesOptions = {}) {
...
}
Hey, thanks for reporting the issue!
To be honest - I'm not the author of this plugin's code and neither spent any significant time to understand it yet. As stated in readme - it's a copy-paste of the internal rule from sonarjs repo because they do not export it directly (you might wanna support the request to publish this rule directly from sonarjs).
So for now I would not be able to do any adjustments to this rule. However I might look into it some time later but not very soon anyway.
Also you are very welcome to try and adjust it yourself if you like and have time =)
Regarding your points.
I agree with your second point that it should not complain about deprecated things that are marked as deprecated, however if you are using them even withing your own code-base I believe it should still report it as that is old API no matter where it's used.
And about first point - I think that even re-exporting code is still considered a usage, because if that code is removed - your code will simply break. So I think that it should still report even re-exports.
Ah that's good to know. I'll see what I can do about it :)
Fair points. But tests import deprecated own-code-functions until they're removed :) The barrels simply re-export own-code-functions, and if that function is part of the public api we still have to export it until the library deprecation policy allows for its removal. So I don't consider it a mistake.
I usually go for a clean lint output as even warnings are a slippery slope. Also if we don't do anything about the warnings why are they on in the first place :) So that's my reasoning
I can partially agree - when you reference/re-export your own deprecated code then yes you might want to remove those from checks as you control this piece of code.
But when you reference/re-export someone else's deprecated code then this is potentially dangerous as you do not control when that code will be removed - so I think it should be reported as the whole point of this plugin is to find and report all places where you rely on deprecated (3rd party) code so you are prepared for next major for example.
But detecting internal/external code boundaries is tricky:
- as you do not always import from relative paths (ex. aliased imports - but still internal)
- also you might re-export some external code internally and then import it somewhere else and it will look like it's internal import while it's not
Also there is a use-case when you develop in monorepo - then suddenly your internal code is not on the same level as you might have different packages that live together but use each other as separate/external packages. And now it's even trickier to decide what to report and what not =)
EDIT: BTW if you are testing your deprecated code - you may simply disable this rule in that test case file
This is very insightful :) It appears this issue is a lot more complex then I initially thought
This issues might be partially fixed by #6 so you might give it a shot in new release.