-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Description
Here is a popular file that breaks (16 million weekly downloads): https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/body-parser/index.d.ts
Failing test case: #2860
The docs say:
eslint-plugin-import/docs/rules/namespace.md
Lines 26 to 28 in cd95728
Currently, this rule does not check for possible | |
redefinition of the namespace in an intermediate scope. Adherence to the ESLint | |
`no-shadow` rule for namespaces will prevent this from being a problem. |
no-shadow
doesn't apply here (it shouldn't - this file successfully compiles and declaration merges under tsc
), since it's a declaration merge and not a shadow. In the case of @types/body-parser
, a variable + namespace declaration merge is necessary (DefinitelyTyped/DefinitelyTyped#57168), since deprecating function overloads is weird in TypeScript: microsoft/TypeScript#40007.
Also, even if it were a shadow, it's a bit harder to adhere when it's in a third-party package.
I could look into adding merging to:
eslint-plugin-import/src/ExportMap.js
Line 306 in cd95728
ExportMap.for = function (context) { |
Actually merging and redeclaration should have the same implementation in this case, since tsc
only allows merging when the namespace has no fields.
Another option could be to deprecate "not found in imported namespace" message since it's a slightly complex feature and I think TypeScript handles it better.