Skip to content

Conversation

eryue0220
Copy link
Contributor

This pr is to remove unused ImportDeclaration check.

@G-Rath
Copy link
Collaborator

G-Rath commented Sep 1, 2024

Can you explain why this isn't needed? It must have been being covered by at least one test so it can't have been unused, though the fact that our suite still passes would suggest it's not needed but it would be good to understand why

@eryue0220
Copy link
Contributor Author

Can you explain why this isn't needed? It must have been being covered by at least one test so it can't have been unused, though the fact that our suite still passes would suggest it's not needed but it would be good to understand why

I have two thoughts about that, the variables importedFunctionsWithSource only collect import elements from import statement, but what it really check is here, another is performance, if we don't use that, but it still iterates the AST nodes to collect something, it's a little worthless.

@G-Rath
Copy link
Collaborator

G-Rath commented Sep 2, 2024

Oh I see, we're only ever writing to importedFunctionsWithSource - we never actually read from it; nice catch!

@G-Rath G-Rath changed the title fix(prefer-importing-jest-globals): remove unused statement check perf(prefer-importing-jest-globals): stop collecting import specifiers for no reason Sep 2, 2024
@G-Rath G-Rath merged commit 0660242 into jest-community:main Sep 2, 2024
44 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 2, 2024
## [28.8.2](v28.8.1...v28.8.2) (2024-09-02)

### Performance Improvements

* **prefer-importing-jest-globals:** stop collecting import specifiers for no reason ([#1646](#1646)) ([0660242](0660242))
Copy link

github-actions bot commented Sep 2, 2024

🎉 This issue has been resolved in version 28.8.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants