This repository was archived by the owner on Jan 19, 2019. It is now read-only.
New: Create option to enable JSXText node type (fixes #266) #272
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 |
JamesHenry
suggested changes
May 18, 2017
tests/lib/jsx.js
Outdated
| "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/invalid-namespace-value-with-dots", // https://github.com/Microsoft/TypeScript/issues/7411 | ||
| "jsx/multiple-blank-spaces" // Fixed by #271 |
There was a problem hiding this comment.
This was fixed, why has it been re-added as one to skip?
There was a problem hiding this comment.
Once I rebase this change I can remove it.
lib/convert.js
Outdated
| @@ -1613,9 +1613,12 @@ module.exports = function convert(config) { | |||
|
|
|||
| } | |||
|
|
|||
| case SyntaxKind.JsxText: | |||
| case SyntaxKind.JsxText: { | |||
There was a problem hiding this comment.
Please can you document inline why we are doing this?
There was a problem hiding this comment.
Sure. It is only temporary until eslint v5, will make a note.
soda0289
force-pushed
the
use-jsxtext-node
branch
from
9be81da to
852eafc
Compare
|
LGTM |
|
@JamesHenry Changes made. Thanks for the review |
|
Ah, I just merged the other JSXText PR, so this will need rebasing. Once that's done this is good to go, and I can see it has already been verified in prettier. |
soda0289
force-pushed
the
use-jsxtext-node
branch
from
852eafc to
76bef55
Compare
|
LGTM |
|
@JamesHenry Rebased. Should be good to go in. |
JamesHenry
approved these changes
May 21, 2017
josephfrazier
added a commit
to josephfrazier/prettier
that referenced
this pull request
May 21, 2017
…strings inside JSXElement eslint/typescript-eslint-parser#272 was merged, so this fixes prettier#1558 See here for more info about the new TSQualifiedName node type: eslint/typescript-eslint-parser@3491b4b
vjeux
pushed a commit
to prettier/prettier
that referenced
this pull request
May 21, 2017
* Run `yarn` to update yarn.lock * Upgrade typescript-eslint-parser, use JSXText instead of Literal for strings inside JSXElement eslint/typescript-eslint-parser#272 was merged, so this fixes #1558 See here for more info about the new TSQualifiedName node type: eslint/typescript-eslint-parser@3491b4b * Run AST comparison tests on Travis
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.
This commit creates a new parser config option
useJSXTextNodethat enablesJSXTextnode type instead ofLiteralfor strings inside ofJSXElementnodes. The default is false as currently many eslint rules depend on theLiteralnode type.I have moved the JSX tests into its own directory and created a new test runner that can run tests for JSX files with and without the
useJSXTextNodeflag enabled.