From 87ae723b527bd8958cf45b47b23f42b28cc48b3d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 8 Sep 2016 09:50:49 -0700 Subject: [PATCH 1/4] For JSX text, construct a single literal node `"foo bar"` instead of `"foo" + " " + "bar"`. --- src/compiler/transformers/jsx.ts | 55 +++++++------------ .../reference/tsxReactEmitWhitespace.js | 6 +- .../reference/tsxReactEmitWhitespace.symbols | 2 +- .../reference/tsxReactEmitWhitespace.types | 2 +- .../jsx/tsxReactEmitWhitespace.tsx | 2 +- 5 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/compiler/transformers/jsx.ts b/src/compiler/transformers/jsx.ts index 9e6aa507cce5a..354c1bcad7d25 100644 --- a/src/compiler/transformers/jsx.ts +++ b/src/compiler/transformers/jsx.ts @@ -151,28 +151,29 @@ namespace ts { } } - function visitJsxText(node: JsxText) { - const text = getTextOfNode(node, /*includeTrivia*/ true); - let parts: Expression[]; + function visitJsxText(node: JsxText): StringLiteral | undefined { + const fixed = fixupWhitespaceAndDecodeEntities(getTextOfNode(node, /*includeTrivia*/ true)); + return fixed !== undefined && createLiteral(fixed); + } + + /** + * JSX trims whitespace at the end and beginning of lines, except that the + * start/end of a tag is considered a start/end of a line only if that line is + * on the same line as the closing tag. See examples in + * tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx + * See also https://www.w3.org/TR/html4/struct/text.html#h-9.1 and https://www.w3.org/TR/CSS2/text.html#white-space-model + */ + function fixupWhitespaceAndDecodeEntities(text: string): string | undefined { + let acc: string | undefined; let firstNonWhitespace = 0; let lastNonWhitespace = -1; - // JSX trims whitespace at the end and beginning of lines, except that the - // start/end of a tag is considered a start/end of a line only if that line is - // on the same line as the closing tag. See examples in - // tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx for (let i = 0; i < text.length; i++) { const c = text.charCodeAt(i); if (isLineBreak(c)) { if (firstNonWhitespace !== -1 && (lastNonWhitespace - firstNonWhitespace + 1 > 0)) { - const part = text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1); - if (!parts) { - parts = []; - } - - // We do not escape the string here as that is handled by the printer - // when it emits the literal. We do, however, need to decode JSX entities. - parts.push(createLiteral(decodeEntities(part))); + const part = decodeEntities(text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1)); + acc = acc === undefined ? part : acc + " " + part; } firstNonWhitespace = -1; @@ -186,28 +187,12 @@ namespace ts { } if (firstNonWhitespace !== -1) { - const part = text.substr(firstNonWhitespace); - if (!parts) { - parts = []; - } - - // We do not escape the string here as that is handled by the printer - // when it emits the literal. We do, however, need to decode JSX entities. - parts.push(createLiteral(decodeEntities(part))); + const lastPart = decodeEntities(text.substr(firstNonWhitespace)); + return acc ? acc + lastPart : lastPart; } - - if (parts) { - return reduceLeft(parts, aggregateJsxTextParts); + else { + return acc; } - - return undefined; - } - - /** - * Aggregates two expressions by interpolating them with a whitespace literal. - */ - function aggregateJsxTextParts(left: Expression, right: Expression) { - return createAdd(createAdd(left, createLiteral(" ")), right); } /** diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.js b/tests/baselines/reference/tsxReactEmitWhitespace.js index dd69091569f30..1588b53d8f4d1 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.js +++ b/tests/baselines/reference/tsxReactEmitWhitespace.js @@ -41,7 +41,7 @@ var p = 0;
; -// Emit "foo" + ' ' + "bar" +// Emit "foo bar"
foo @@ -75,5 +75,5 @@ React.createElement("div", null, " 3 "); React.createElement("div", null, "3"); // Emit no args React.createElement("div", null); -// Emit "foo" + ' ' + "bar" -React.createElement("div", null, "foo" + " " + "bar"); +// Emit "foo bar" +React.createElement("div", null, "foo bar"); diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.symbols b/tests/baselines/reference/tsxReactEmitWhitespace.symbols index a0d8266faa0e1..4639e828b5268 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.symbols +++ b/tests/baselines/reference/tsxReactEmitWhitespace.symbols @@ -79,7 +79,7 @@ var p = 0;
; >div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) -// Emit "foo" + ' ' + "bar" +// Emit "foo bar"
>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.types b/tests/baselines/reference/tsxReactEmitWhitespace.types index 824aa5cfdae99..ce6911f987471 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.types +++ b/tests/baselines/reference/tsxReactEmitWhitespace.types @@ -88,7 +88,7 @@ var p = 0;
; >div : any -// Emit "foo" + ' ' + "bar" +// Emit "foo bar"
>
foo bar
: JSX.Element >div : any diff --git a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx index 34fd158eab196..9977803e3956d 100644 --- a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx +++ b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx @@ -42,7 +42,7 @@ var p = 0;
; -// Emit "foo" + ' ' + "bar" +// Emit "foo bar"
foo From 9ae98d6a378805661f802974f3794ce5481f1529 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 8 Sep 2016 12:49:58 -0700 Subject: [PATCH 2/4] Fix bug: return `undefined`, not `false` --- src/compiler/transformers/jsx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/transformers/jsx.ts b/src/compiler/transformers/jsx.ts index 354c1bcad7d25..2a18adeda0e85 100644 --- a/src/compiler/transformers/jsx.ts +++ b/src/compiler/transformers/jsx.ts @@ -153,7 +153,7 @@ namespace ts { function visitJsxText(node: JsxText): StringLiteral | undefined { const fixed = fixupWhitespaceAndDecodeEntities(getTextOfNode(node, /*includeTrivia*/ true)); - return fixed !== undefined && createLiteral(fixed); + return fixed === undefined ? undefined : createLiteral(fixed); } /** From aa322ea18ae247e63eec6e6ae9d828e61525c1a5 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 9 Sep 2016 06:32:45 -0700 Subject: [PATCH 3/4] Add test with backslash --- tests/baselines/reference/tsxReactEmitWhitespace.js | 9 +++++++++ .../baselines/reference/tsxReactEmitWhitespace.symbols | 9 +++++++++ tests/baselines/reference/tsxReactEmitWhitespace.types | 10 ++++++++++ tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx | 7 +++++++ 4 files changed, 35 insertions(+) diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.js b/tests/baselines/reference/tsxReactEmitWhitespace.js index 1588b53d8f4d1..f8bdead81d25a 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.js +++ b/tests/baselines/reference/tsxReactEmitWhitespace.js @@ -50,6 +50,13 @@ var p = 0;
; +// Emit "hello\\ world" +
+ + hello\ + +world +
; //// [file.js] @@ -77,3 +84,5 @@ React.createElement("div", null, "3"); React.createElement("div", null); // Emit "foo bar" React.createElement("div", null, "foo bar"); +// Emit "hello\\ world" +React.createElement("div", null, "hello\\ world"); diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.symbols b/tests/baselines/reference/tsxReactEmitWhitespace.symbols index 4639e828b5268..bb04dbf04b9e0 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.symbols +++ b/tests/baselines/reference/tsxReactEmitWhitespace.symbols @@ -90,4 +90,13 @@ var p = 0;
; >div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) +// Emit "hello\\ world" +
+>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) + + hello\ + +world +
; +>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.types b/tests/baselines/reference/tsxReactEmitWhitespace.types index ce6911f987471..25aed60f647b2 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.types +++ b/tests/baselines/reference/tsxReactEmitWhitespace.types @@ -100,4 +100,14 @@ var p = 0; ; >div : any +// Emit "hello\\ world" +
+>
hello\world
: JSX.Element +>div : any + + hello\ + +world +
; +>div : any diff --git a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx index 9977803e3956d..4853a504744cf 100644 --- a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx +++ b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx @@ -51,3 +51,10 @@ var p = 0; ; +// Emit "hello\\ world" +
+ + hello\ + +world +
; From 31669504b9e94e452b053c22d6d37834fd13ed34 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 9 Sep 2016 07:56:01 -0700 Subject: [PATCH 4/4] Comment code and remember to add a space before the last part --- src/compiler/transformers/jsx.ts | 43 ++++++++++++++----- .../reference/tsxReactEmitWhitespace.js | 7 +++ .../reference/tsxReactEmitWhitespace.symbols | 8 ++++ .../reference/tsxReactEmitWhitespace.types | 9 ++++ .../jsx/tsxReactEmitWhitespace.tsx | 5 +++ 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/compiler/transformers/jsx.ts b/src/compiler/transformers/jsx.ts index 2a18adeda0e85..556c26c37f049 100644 --- a/src/compiler/transformers/jsx.ts +++ b/src/compiler/transformers/jsx.ts @@ -162,23 +162,39 @@ namespace ts { * on the same line as the closing tag. See examples in * tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx * See also https://www.w3.org/TR/html4/struct/text.html#h-9.1 and https://www.w3.org/TR/CSS2/text.html#white-space-model + * + * An equivalent algorithm would be: + * - If there is only one line, return it. + * - If there is only whitespace (but multiple lines), return `undefined`. + * - Split the text into lines. + * - 'trimRight' the first line, 'trimLeft' the last line, 'trim' middle lines. + * - Decode entities on each line (individually). + * - Remove empty lines and join the rest with " ". */ function fixupWhitespaceAndDecodeEntities(text: string): string | undefined { let acc: string | undefined; + // First non-whitespace character on this line. let firstNonWhitespace = 0; + // Last non-whitespace character on this line. let lastNonWhitespace = -1; + // These initial values are special because the first line is: + // firstNonWhitespace = 0 to indicate that we want leading whitsepace, + // but lastNonWhitespace = -1 as a special flag to indicate that we *don't* include the line if it's all whitespace. for (let i = 0; i < text.length; i++) { const c = text.charCodeAt(i); if (isLineBreak(c)) { - if (firstNonWhitespace !== -1 && (lastNonWhitespace - firstNonWhitespace + 1 > 0)) { - const part = decodeEntities(text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1)); - acc = acc === undefined ? part : acc + " " + part; + // If we've seen any non-whitespace characters on this line, add the 'trim' of the line. + // (lastNonWhitespace === -1 is a special flag to detect whether the first line is all whitespace.) + if (firstNonWhitespace !== -1 && lastNonWhitespace !== -1) { + acc = addLineOfJsxText(acc, text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1)); } + // Reset firstNonWhitespace for the next line. + // Don't bother to reset lastNonWhitespace because we ignore it if firstNonWhitespace = -1. firstNonWhitespace = -1; } - else if (!isWhiteSpace(c)) { + else if (!isWhiteSpaceSingleLine(c)) { lastNonWhitespace = i; if (firstNonWhitespace === -1) { firstNonWhitespace = i; @@ -186,13 +202,18 @@ namespace ts { } } - if (firstNonWhitespace !== -1) { - const lastPart = decodeEntities(text.substr(firstNonWhitespace)); - return acc ? acc + lastPart : lastPart; - } - else { - return acc; - } + return firstNonWhitespace !== -1 + // Last line had a non-whitespace character. Emit the 'trimLeft', meaning keep trailing whitespace. + ? addLineOfJsxText(acc, text.substr(firstNonWhitespace)) + // Last line was all whitespace, so ignore it + : acc; + } + + function addLineOfJsxText(acc: string | undefined, trimmedLine: string): string { + // We do not escape the string here as that is handled by the printer + // when it emits the literal. We do, however, need to decode JSX entities. + const decoded = decodeEntities(trimmedLine); + return acc === undefined ? decoded : acc + " " + decoded; } /** diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.js b/tests/baselines/reference/tsxReactEmitWhitespace.js index f8bdead81d25a..548394125677d 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.js +++ b/tests/baselines/reference/tsxReactEmitWhitespace.js @@ -57,6 +57,11 @@ var p = 0; world ; + +// Emit " a b c d " +
a + b c + d
; //// [file.js] @@ -86,3 +91,5 @@ React.createElement("div", null); React.createElement("div", null, "foo bar"); // Emit "hello\\ world" React.createElement("div", null, "hello\\ world"); +// Emit " a b c d " +React.createElement("div", null, " a b c d "); diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.symbols b/tests/baselines/reference/tsxReactEmitWhitespace.symbols index bb04dbf04b9e0..baac01227b6c2 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.symbols +++ b/tests/baselines/reference/tsxReactEmitWhitespace.symbols @@ -100,3 +100,11 @@ world ; >div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) +// Emit " a b c d " +
a +>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) + + b c + d
; +>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22)) + diff --git a/tests/baselines/reference/tsxReactEmitWhitespace.types b/tests/baselines/reference/tsxReactEmitWhitespace.types index 25aed60f647b2..46ce9f305b4c7 100644 --- a/tests/baselines/reference/tsxReactEmitWhitespace.types +++ b/tests/baselines/reference/tsxReactEmitWhitespace.types @@ -111,3 +111,12 @@ world ; >div : any +// Emit " a b c d " +
a +>
a b c d
: JSX.Element +>div : any + + b c + d
; +>div : any + diff --git a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx index 4853a504744cf..be677b37ff893 100644 --- a/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx +++ b/tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx @@ -58,3 +58,8 @@ var p = 0; world ; + +// Emit " a b c d " +
a + b c + d
;