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.

Sign up for GitHub

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

Jump to bottom

Template token objects differ from other token objects #319

Closed
iancmyers opened this issue Feb 17, 2017 · 3 comments · Fixed by #343
Closed

Template token objects differ from other token objects #319

iancmyers opened this issue Feb 17, 2017 · 3 comments · Fixed by #343
Labels
accepted bug

Comments

@iancmyers
Copy link
Contributor

iancmyers commented Feb 17, 2017 •
edited

While fixing an issue in eslint-plugin-react (jsx-eslint/eslint-plugin-react#1061), I noticed that template tokens differ from other tokens. A typical token looks like this:

Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }

A template token looks like this:

{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }

I've not be able to figure out why templates are plain JavaScript objects and not of the class type Token (aside from the fact that the template tokens are constructed differently in the token translator).

The more pressing issue, in my mind, is the omission of start and end properties at the top level of the template token object. The issue cited above was caused by passing token.start into eslint's getNodeByRangeIndex method. I fixed the issue by using range[0] (jsx-eslint/eslint-plugin-react#1077), however I fear that other downstream projects are relying on tokens have a consistent object construction and always having start and end.

It seems like the quick-fix would be to copy the range values into start and end properties on the template tokens. If that would be sufficient for now, I'd be happy to PR that.
@nzakas
Copy link
Member

nzakas commented Feb 23, 2017

That seems like a reasonable fix for what appears to be an obvious bug. Probably just an oversight as we went back and forth on whether or not start and end should end up on the nodes. I'd still recommend using range in rules, as that is supported by all parsers whereas start and end are just Acorn leftovers in Espree.

@kaicataldo
Copy link
Member

@iancmyers Sorry for losing track of this. Is this still something you'd be willing to make a PR for?

@iancmyers
Copy link
Contributor Author

Yep! Incoming soon.

iancmyers added a commit to iancmyers/espree that referenced this issue Jun 19, 2017
@iancmyers
Make template token objects adhere token object structure
cbe68e7 
While fixing an issue in `eslint-plugin-react`
(jsx-eslint/eslint-plugin-react#1061),
I noticed that template tokens differ from other tokens. A typical token
looks like this:

```
Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

A template token looks like this:

```
{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

I've not be able to figure out why templates are plain JavaScript
objects and not of the class type `Token` (aside from the fact that the
template tokens are constructed differently in the token translator.

This fix copies the `range` values into `start` and `end` properties on
the template tokens to make them adhere to the same structure as other
token objects.

Fixes eslint#319
@iancmyers iancmyers mentioned this issue Jun 19, 2017
Merged
iancmyers added a commit to iancmyers/espree that referenced this issue Jun 21, 2017
@iancmyers
Make template token objects adhere token object structure
fd4c2ac 
While fixing an issue in `eslint-plugin-react`
(jsx-eslint/eslint-plugin-react#1061),
I noticed that template tokens differ from other tokens. A typical token
looks like this:

```
Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

A template token looks like this:

```
{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

I've not be able to figure out why templates are plain JavaScript
objects and not of the class type `Token` (aside from the fact that the
template tokens are constructed differently in the token translator.

This fix copies the `range` values into `start` and `end` properties on
the template tokens to make them adhere to the same structure as other
token objects.

Fixes eslint#319
iancmyers added a commit to iancmyers/espree that referenced this issue Jun 21, 2017
@iancmyers
Make template token objects adhere token object structure
289a848 
While fixing an issue in `eslint-plugin-react`
(jsx-eslint/eslint-plugin-react#1061),
I noticed that template tokens differ from other tokens. A typical token
looks like this:

```
Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

A template token looks like this:

```
{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

I've not be able to figure out why templates are plain JavaScript
objects and not of the class type `Token` (aside from the fact that the
template tokens are constructed differently in the token translator.

This fix copies the `range` values into `start` and `end` properties on
the template tokens to make them adhere to the same structure as other
token objects.

Fixes eslint#319
not-an-aardvark pushed a commit that referenced this issue Jul 10, 2017
@iancmyers @not-an-aardvark
Make template token objects adhere token object structure (#343)
3805aea 
While fixing an issue in `eslint-plugin-react`
(jsx-eslint/eslint-plugin-react#1061),
I noticed that template tokens differ from other tokens. A typical token
looks like this:

```
Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

A template token looks like this:

```
{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

I've not be able to figure out why templates are plain JavaScript
objects and not of the class type `Token` (aside from the fact that the
template tokens are constructed differently in the token translator.

This fix copies the `range` values into `start` and `end` properties on
the template tokens to make them adhere to the same structure as other
token objects.

Fixes #319
not-an-aardvark pushed a commit that referenced this issue Jul 10, 2017
@iancmyers @not-an-aardvark
Fix: Make template token objects adhere to token object structure (#343)
d4bdcb6 
While fixing an issue in `eslint-plugin-react`
(jsx-eslint/eslint-plugin-react#1061),
I noticed that template tokens differ from other tokens. A typical token
looks like this:

```
Token {
  type: 'String',
  value: '"bar"',
  start: 44,
  end: 49,
  loc:
   SourceLocation {
     start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

A template token looks like this:

```
{ type: 'Template',
  value: '`bar`',
  loc:
   { start: Position { line: 4, column: 11 },
     end: Position { line: 4, column: 16 } },
  range: [ 44, 49 ] }
```

I've not be able to figure out why templates are plain JavaScript
objects and not of the class type `Token` (aside from the fact that the
template tokens are constructed differently in the token translator.

This fix copies the `range` values into `start` and `end` properties on
the template tokens to make them adhere to the same structure as other
token objects.

Fixes #319
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make template token objects adhere token object structure iancmyers/espree
4 participants
@nzakas @iancmyers @kaicataldo @eslintbot