-
-
Notifications
You must be signed in to change notification settings - Fork 234
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: if LEAST_UPPER_BOUND returns null, try GREATEST_LOWER_BOUND #375
Conversation
sourceMap, | ||
generatedLocation.start.line, | ||
generatedLocation.start.column | ||
); | ||
let end = originalEndPositionFor(sourceMap, generatedLocation.end); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe, in the case of my testing with c8
, it was the GREATEST_LOWER_BOUND
in the originalEndPositionFor
that was resulting in null
data; opting for LEAST_UPPER_BOUND
works like a charm ... but I don't fully understand why both wouldn't return an element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I haven't really had a chance to gain a reasonable understanding of source-map modules / API's. What I can say is that I've confirmed that this patch combined with istanbuljs/nyc#1080 resolve some issues which were raised starting at nyc@13.2.0.
bias: GREATEST_LOWER_BOUND | ||
}); | ||
if (mapping.source === null) { | ||
mapping = sourceMap.originalPositionFor({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: You could just return sourceMap.originalPositionFor({
here then mapping
could be const.
I dug into the If you have // we are in termination case (3) or (2) and return the appropriate thing.
if (aBias == exports.LEAST_UPPER_BOUND) {
return mid;
} else {
return aLow < 0 ? -1 : aLow;
} inversely, if if (aBias == exports.LEAST_UPPER_BOUND) {
return aHigh < aHaystack.length ? aHigh : -1;
} else {
return mid;
} By trying a search for There's a similar conversation happening here about trying both approaches: |
I'm running into issues adding source-map support to c8, in which
LEAST_UPPER_BOUND
returnsmapping.source === null
.If I loosen up our check so that it falls back to the default behavior of
GREATEST_LOWER_BOUND
in this edge-case, c8's remapping works like a charm, see:@loganfsmyth can you clarify for me why this change might address my issues? I understand that they underlying cause is most likely that V8's coverage ranges don't directly map to the mappings generated by TypeScript ... but why would
LEAST_UPPER_BOUND
be returningnull
?Reading the documentation, shouldn't it simply return the next closest element that's a greater line number than the element I'm remapping -- I guess I don't fully understand the scenarios that result in
LEAST_UPPER_BOUND
orGREATEST_LOWER_BOUND
returningnull
.