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: handle end out of range errors #199

Closed
wants to merge 3 commits into from

Conversation

hurstindustries
Copy link

@hurstindustries hurstindustries commented Nov 14, 2022

Fixes issue Cannot read properties of undefined (reading 'endCol') in lib/range.js

See comment by @devcorraliza on #198 (comment) for context

@SimenB
Copy link
Member

SimenB commented Nov 14, 2022

Please include a test 🙂

@SimenB SimenB requested a review from bcoe November 14, 2022 22:48
@hurstindustries hurstindustries changed the title Fix issue with end out of range error fix: handle end out of range errors Nov 14, 2022
@devcorraliza
Copy link

Hi, any pending task to review/merge?

@wolandec
Copy link

Hi, could you please release this change?

@SimenB
Copy link
Member

SimenB commented Dec 1, 2022

@jridgewell does this seem reasonable to you?

Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

I think this fix is in the wrong location, because it's so far from the actual spot the bug is introduced. I would update

end = originalPositionFor(sourceMap, {
line: lines[lines.length - 1].line,
column: endCol - lines[lines.length - 1].startCol,
bias: LEAST_UPPER_BOUND
})
end.column -= 1

      const maybeEnd = originalPositionFor(sourceMap, {
        line: lines[lines.length - 1].line,
        column: endCol - lines[lines.length - 1].startCol,
        bias: LEAST_UPPER_BOUND
      })
      if (maybeEnd) {
        end = maybeEnd
        end.column -= 1
      }

Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Actually, this seems to be fixing an unrelated issue? The case I debugged in #198 does not hit this error case. I'm not sure how this case is hit.

@wolandec
Copy link

wolandec commented Dec 1, 2022

The changes in this PR fix that reading 'endCol' error in our project.

This works for us too lines[mid]?.endCol . Hope it helps.

while (mid >= 0 && startCol < lines[mid].endCol && endCol >= lines[mid].startCol) {

      while (mid >= 0 && startCol < lines[mid]?.endCol && endCol >= lines[mid].startCol) {

@bcoe
Copy link
Member

bcoe commented Feb 14, 2023

Not sure what the best solution is here between the options presented by @jridgewell, @wolandec, and @hurstindustries -- open to suggestions from someone who's dug into this deeply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants