Fix: Ignore re-export all in no-duplicate-imports (fixes #12760)

[mdjermanovic]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix #12760

What changes did you make? (Give an overview)

After this PR, no-duplicate-imports will ignore export * from "mod", since it isn't possible to merge these exports with any other imports/exports.

I believe it's actually even impossible to rewrite such code in any other way, so it seems that taking into account these declarations was a bug.

Also, documentation was missing the explanation for the includeExports option. There was just a special case with both import and export declarations.

Is there anything you'd like reviewers to focus on?

Yes. Ignoring export * from will also ignore duplicates:

export * from "mod";
export * from "mod";

This looks like a possible error in exporting rather than a stylistic issue in importing (which is what this rule is about?). So, this indeed might not belong here, but after this fix there will be no rule to report it.

[kaicataldo]

Thanks for working on this!

Yes. Ignoring export * from will also ignore duplicates:

export * from "mod";
export * from "mod";

This looks like a possible error in exporting rather than a stylistic issue in importing (which is what this rule is about?). So, this indeed might not belong here, but after this fix there will be no rule to report it.

I hadn't considered this. I wonder if it would make sense to still warn on this case, since it really is a duplicate?

[G-Rath]

Yes. Ignoring export * from will also ignore duplicates:
I wonder if it would make sense to still warn on this case, since it really is a duplicate?

I think it makes a lot of sense, as it's entire reason for enabling the rule 🙂

I think this ties in closely to #12758, and that the implementation I believe will be required for that would also solve this: effectively, no-duplicate-imports needs to construct a collection of all the imports by their types, and then do it's checks based on those types, where as right now it just collects all imports and compares.

This logic would be the same for handling exports, with a minor adjustment for namespace exports.

I'm happy to implement this, or for @mdjermanovic to do so, but I think only one of us should so that we end up with nicely mirrored code for both export & import.

[mdjermanovic]

I thought export * is a separate problem as it just needs to be removed from this rule, but then the issue with duplicates of itself came up.

Agreed, let's discuss everything in #12758 first. If it turns out that this doesn't make sense as a separate fix, I'll close it.

[nzakas]

@kaicataldo @mdjermanovic thoughts on how to proceed here?

[mdjermanovic]

I agree with @G-Rath: it would be best to fix problems described in #12758 and #12760 in one PR.

I think we should close this PR and update #12758 issue with a note saying that a PR for it should also fix #12760 (+ account for the new export * as ns syntax).

[nzakas]

I agree with @G-Rath: it would be best to fix problems described in #12758 and #12760 in one PR.

I think we should close this PR and update #12758 issue with a note saying that a PR for it should also fix #12760 (+ account for the new export * as ns syntax).

Agreed. Closing.