fix: calculate highlight offset using stripped paths #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the Jest rootDir is not the same as the package's root directory (i.e., a monorepo scenario with multiple local Jest configs) the highlight function incorrectly calculates the offset for highlighting file name matches, resulting in a highlight in the wrong place. This only occurs when filePath is not truncated.
This change fixes the failing test by moving the offset calculation out of the conditional and calculating it the same way in all scenarios using rawPath and filePath.
Since highlight no longer needs rootDir to calculate the highlight offset, this change removes rootDir from highlight's parameters. It also updates tests to use inline snapshots now that there are fewer scenarios to account for.
The changes in this PR allow us to remove the rootDir param from the highlight function. I forgot to remove it from the function declaration and it was causing type checking to fail.
Uses separate test functions and inline snapshots to make tests easier to reason about.
|
Thanks, let us know once this is out of draft status |
|
@jeysal Ready for review 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a potential fix to #46.
I'm working on a way to make the tests more accurate so that they'll actually fail when the highlight placement is incorrect. Even though I've been able to visually confirm that this one-line change seems to resolve the issue, the unit tests I've included pass both with and without the change, so they're not useful until I can pass an ANSI-formatted string (done by
chalk) as thefilePathparameter to thehighlightfunction. It does work to just console.log an actual filePath that was formatted bychalk, and then use that as the file path in tests, but the hard-coded string is cluttered with ANSI codes and is really confusing 😅As a side note, I figured out what went wrong with #34. It looks like when I resolved merge conflicts from #41 I missed the name change of
rawPathandfilePathtostrippedRawPathandstrippedFilePath. The tests still passed because they were already passing stripped strings as thefilePathparameter.