Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

Fix: Optimize convertTokens, treat JsxText as token (fixes #70) #158

Merged
merged 1 commit into from
Feb 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 34 additions & 10 deletions lib/ast-converter.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,26 @@ function fixExports(node, result, ast) {
return result;
}

/**
* Returns true if a given TSNode is a token
* @param {TSNode} node the TSNode
* @returns {boolean} is a token
*/
function isToken(node) {
/**
* Note: We treat JsxText like a token (TypeScript doesn't classify it as one),
Copy link

@DanielRosenwasser DanielRosenwasser Feb 20, 2017

Choose a reason for hiding this comment

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

This sounds like a bug on us. I vaguely recall trying to fix this months ago but it was not a simple fix if it's the same issue I was thinking about - basically there were some invariants violated in the language service as a result.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, and thanks for keeping an eye on the project @DanielRosenwasser 😄

I will take another look at this once TypeScript 2.2 hits stable.

Copy link
Member Author

Choose a reason for hiding this comment

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

@DanielRosenwasser just FYI this does indeed work as of TS 2.2!

I am removing this extra check in the PR to support 2.2 -> #169

* to prevent traversing into it and creating unintended addiontal tokens from
* its contents.
*
* It looks like this has been addressed in the master branch of TypeScript, so we can
* look to remove this extra check as part of the 2.2.x support
*/
if (node.kind === ts.SyntaxKind.JsxText) {
return true;
}
return node.kind >= ts.SyntaxKind.FirstToken && node.kind <= ts.SyntaxKind.LastToken;
}

/**
* Returns true if a given TSNode is a JSX token
* @param {TSNode} node TSNode to be checked
Expand Down Expand Up @@ -421,18 +441,22 @@ function convertToken(token, ast) {
* @returns {ESTreeToken[]} the converted ESTreeTokens
*/
function convertTokens(ast) {
var token = ast.getFirstToken(),
converted,
result = [];

while (token) {
converted = convertToken(token, ast);
if (converted) {
result.push(converted);
var result = [];
/**
* @param {TSNode} node the TSNode
* @returns {undefined}
*/
function walk(node) {
if (isToken(node) && node.kind !== ts.SyntaxKind.EndOfFileToken) {
var converted = convertToken(node, ast);
if (converted) {
result.push(converted);
}
} else {
node.getChildren().forEach(walk);
}
token = ts.findNextToken(token, ast);
}

walk(ast);
return result;
}

Expand Down
1 change: 0 additions & 1 deletion tests/lib/ecma-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ var filesWithOutsandingTSIssues = [
"jsx/embedded-tags", // https://github.com/Microsoft/TypeScript/issues/7410
"jsx/namespaced-attribute-and-value-inserted", // https://github.com/Microsoft/TypeScript/issues/7411
"jsx/namespaced-name-and-attribute", // https://github.com/Microsoft/TypeScript/issues/7411
"jsx/test-content", // https://github.com/Microsoft/TypeScript/issues/7471
"jsx/multiple-blank-spaces"
];

Expand Down