Skip to content
This repository has been archived by the owner on Dec 17, 2023. It is now read-only.

feat(core): Add config option to skip files considered for imports #140

Merged
merged 6 commits into from
Aug 23, 2021
Merged

feat(core): Add config option to skip files considered for imports #140

merged 6 commits into from
Aug 23, 2021

Conversation

sauntimo
Copy link
Contributor

@sauntimo sauntimo commented Aug 7, 2021 •
edited
Loading

Background

  • closes [Question] also find dead code only used in tests #136
  • When functions are imported into tests, (and depending on the local tsconfig settings), ts-prune may no longer consider them dead code, despite them not being use "in anger" anywhere in the project. In these instances it would be useful to be able to specify a pattern (eg .test.ts) whereby matching source files would be ignored when determining if code is dead.

Changes

  • adds -s --skip [regexp] flag which allows users to specify a pattern for source files to ignore
  • adds two tests for getPotentiallyUnused to ensure that it's working with and without a skip pattern
  • adds a skip pattern case to the integration test suite
  • fixes delete command and gitignore for the outfile generated by the integration test, and deletes it from git history.

nadeesha
nadeesha reviewed Aug 7, 2021
View reviewed changes
Copy link
Owner

@nadeesha nadeesha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Almost there.

Could you add a test case to the integration tests as well?

src/analyzer.ts Outdated Show resolved Hide resolved
nadeesha
nadeesha reviewed Aug 7, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
nadeesha
nadeesha reviewed Aug 7, 2021
View reviewed changes
src/configurator.ts Outdated Show resolved Hide resolved
@sauntimo
Copy link
Contributor Author

sauntimo commented Aug 7, 2021

Could you add a test case to the integration tests as well?

Ah yeah, good call. I'll have a look at this in the week. Thanks for your comments, they all make sense to me 👍

@sauntimo
Copy link
Contributor Author

sauntimo commented Aug 9, 2021

@nadeesha Hopefully I've addressed your feedback, thanks for reviewing! I just wanted to check about the integration test; ts-prune is only run once and I've add a --skip option to this - is that alright? I figured breaking it out into multiple runs was out of scope for now.

@sauntimo sauntimo requested a review from nadeesha August 9, 2021 20:54
@sauntimo
Copy link
Contributor Author

@nadeesha hey, sorry I appreciate you're probably busy but I'd love to get this finished off if you get a moment 😁 No worries if you don't get a chance to look at it though. Have a nice weekend!

nadeesha
nadeesha reviewed Aug 20, 2021
View reviewed changes
Copy link
Owner

@nadeesha nadeesha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sauntimo Excellent. I just took a look and almost everything looks great.

I saw however that the integration tests are failing. Could you kindly look into that, and we can merge it right after?

@sauntimo
Copy link
Contributor Author

Oh, that's embarrassing! Thanks for pointing that out, I'll see if can fix this over the weekend.

@sauntimo
Copy link
Contributor Author

@nadeesha I think I've fixed the integration test now. I noticed that the generated outfile had somehow gotten git tracked so I updated the command to remove it, and the gitignore which should have stopped it from being tracked. I realised that I had added my case to that outfile rather than the outfile.base 🤦. Hopefully everything is working now!

@nadeesha nadeesha changed the title Add option to skip src files with pattern feat(core): Add config option to skip files considered for imports Aug 23, 2021
@nadeesha nadeesha merged commit 8a1b2f8 into nadeesha:master Aug 23, 2021
@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nadeesha
Copy link
Owner

@sauntimo Thanks for addressing the feedback. This is now merged.

@sauntimo
Copy link
Contributor Author

@nadeesha awesome - thanks for your patience & help with it! ☺️

@jtbandes jtbandes mentioned this pull request Aug 25, 2021
Merged
jtbandes added a commit to foxglove/studio that referenced this pull request Aug 26, 2021
@jtbandes
Add ts-prune --skip flag and delete code that's only used in tests/st…
3f162f4 
…ories (#1701)

**User-Facing Changes**
None

**Description**
ts-prune has a new `--skip` flag (nadeesha/ts-prune#140) that allows us to ignore usages from `.test.ts` and `.stories.tsx` files when determining whether code is used.

There are some false positives (code that is _intended_ to be used only in tests) that have been suppressed with `// ts-prune-ignore-next`. I think ts-prune could be enhanced to allow annotating code as test-only without outright ignoring it; added a comment to this effect on nadeesha/ts-prune#136 (comment).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nadeesha nadeesha nadeesha left review comments

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Question] also find dead code only used in tests
2 participants
@sauntimo @nadeesha