This repository was archived by the owner on Jan 19, 2019. It is now read-only.
Fix: Use correct starting range and loc for JSXText tokens (fixes #227) #271
Conversation
This file contains hidden or 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
|
LGTM |
soda0289
force-pushed
the
fix-jsxtext-token
branch
from
8ead954 to
42f1f3e
Compare
|
LGTM |
JamesHenry
reviewed
May 15, 2017
lib/node-utils.js
Outdated
| @@ -588,15 +588,16 @@ function getTokenType(token) { | |||
| * @returns {ESTreeToken} the converted ESTreeToken | |||
| */ | |||
| function convertToken(token, ast) { | |||
| const start = token.getStart(), | |||
| const start = (token.kind === SyntaxKind.JsxText) ? token.getFullStart() : token.getStart(), | |||
| end = token.getEnd(), | |||
| value = ast.text.slice(start, token.end), | |||
There was a problem hiding this comment.
All the other references to token.end were updated to the variable end, but not this one. Just checking that is intended/not significant?
There was a problem hiding this comment.
I was reading through the code of typescript and noticed this method. I thought it looked a little cleaner since we use methods to access the starting point.
These are the methods to access token positions:
getFullStart()is the same as.posgetEnd()is the same as.endgetStart()is the same as.posbut skipping over trivia and white space
If you want I can change the logic to use the properties to be more consistent with the rest of the code.
soda0289
force-pushed
the
fix-jsxtext-token
branch
from
42f1f3e to
f190876
Compare
|
LGTM |
JamesHenry
approved these changes
May 15, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
We used to use the method
getStart()to calculate the starting range for JSXText tokens. This would skip over whitespace and cause the value of these tokens to be empty. This would also result in the range and starting line number and column to be incorrect. This commit uses the methodgetFullStart(), on JSXText tokens, which does not skip over whitespace and allows for the correct range, value and loc to be set.This also fixes issues with JSX indentation in the new indent rule and in the eslint-plugin-react.