Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix counting re-export as usage when used in combination with import #1727

Merged
merged 1 commit into from
Apr 14, 2020

Conversation

Ephem
Copy link
Contributor

@Ephem Ephem commented Apr 13, 2020

Closes #1722

This PR makes sure aa below is correctly counted as usage:

import { a } from './a';
export { aa } from './a';

In addition to the attached test, this fix was tested against a previous false negative in a private project which now worked.

I found the existing tests quite hard to read and understand, so I went a bit off the beaten path and tried to isolate this test in new files to avoid coupling them too much to the existing ones. I also named the files descriptively as opposed to following the existing standard of letters. This was just best effort after finally grokking what was going on, so I'm happy to take any pointers on how to improve or tweak!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.808% when pulling 40d1e67 on Ephem:1722-fix-re-export-usage into 3b4487c on benmosher:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.808% when pulling 40d1e67 on Ephem:1722-fix-re-export-usage into 3b4487c on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.808% when pulling 40d1e67 on Ephem:1722-fix-re-export-usage into 3b4487c on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.808% when pulling 40d1e67 on Ephem:1722-fix-re-export-usage into 3b4487c on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.808% when pulling 40d1e67 on Ephem:1722-fix-re-export-usage into 3b4487c on benmosher:master.

@ljharb
Copy link
Member

ljharb commented Apr 14, 2020

New fixture files are great, thanks :-)

@ljharb ljharb merged commit 40d1e67 into import-js:master Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[no-unused-modules] Imports overriding reexports
3 participants