From 08e077c0e05b7c05e33551be7727b62429a9dbbc Mon Sep 17 00:00:00 2001 From: George Vrettos Date: Wed, 4 Nov 2020 21:49:29 +0200 Subject: [PATCH 01/11] docs: fix typo in the "Property Binding" guide (#39569) PR Close #39569 --- aio/content/guide/property-binding.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aio/content/guide/property-binding.md b/aio/content/guide/property-binding.md index c9240da7aec4..d85778ef6117 100644 --- a/aio/content/guide/property-binding.md +++ b/aio/content/guide/property-binding.md @@ -2,7 +2,7 @@ # Property binding Property binding in Angular helps you set values for properties of HTML elements or directives. -With property binding, you can do things such as toggle button functionality, set paths programatically, and share values between components. +With property binding, you can do things such as toggle button functionality, set paths programmatically, and share values between components.
From e67a331b12a535c3d3e55c4ea5904c2e1b1c1757 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 28 Oct 2020 21:01:31 +0000 Subject: [PATCH 02/11] fix(compiler): ensure that i18n message-parts have the correct source-span (#39589) In an i18n message, two placeholders next to each other must have an "empty" message-part to separate them. Previously, the source-span for this message-part was pointing to the wrong original location. This caused problems in the generated source-maps and lead to extracted i18n messages from being rendered incorrectly. PR Close #39589 --- .../src/render3/view/i18n/localize_utils.ts | 2 +- .../compiler/test/render3/view/i18n_spec.ts | 26 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts index 444139ecacb6..b57593e4d855 100644 --- a/packages/compiler/src/render3/view/i18n/localize_utils.ts +++ b/packages/compiler/src/render3/view/i18n/localize_utils.ts @@ -120,7 +120,7 @@ function processMessagePieces(pieces: o.MessagePiece[]): placeHolders.push(part); if (pieces[i - 1] instanceof o.PlaceholderPiece) { // There were two placeholders in a row, so we need to add an empty message part. - messageParts.push(createEmptyMessagePart(part.sourceSpan.end)); + messageParts.push(createEmptyMessagePart(pieces[i - 1].sourceSpan.end)); } } } diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index 50d534c64ea8..339705f1beee 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -458,6 +458,26 @@ describe('serializeI18nMessageForLocalize', () => { expect(placeHolders[3].sourceSpan.toString()).toEqual(''); }); + it('should create the correct source-spans when there are two placeholders next to each other', + () => { + const {messageParts, placeHolders} = serialize('{{value}}'); + expect(messageParts[0].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)'); + expect(messageParts[1].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('"" (13-13)'); + expect(messageParts[2].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (22-22)'); + expect(messageParts[3].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (26-26)'); + + expect(placeHolders[0].text).toEqual('START_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"" (10-13)'); + expect(placeHolders[1].text).toEqual('INTERPOLATION'); + expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (13-22)'); + expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"" (22-26)'); + }); + it('should serialize simple ICU for `$localize()`', () => { expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({ messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')], @@ -465,7 +485,6 @@ describe('serializeI18nMessageForLocalize', () => { }); }); - it('should serialize nested ICUs for `$localize()`', () => { expect(serialize( '{age, plural, 10 {ten {size, select, 1 {one} 2 {two} other {2+}}} other {other}}')) @@ -478,7 +497,6 @@ describe('serializeI18nMessageForLocalize', () => { }); }); - it('should serialize ICU with embedded HTML for `$localize()`', () => { expect(serialize('{age, plural, 10 {ten} other {
other
}}')).toEqual({ messageParts: [ @@ -564,3 +582,7 @@ function literal(text: string, span: any = jasmine.any(ParseSourceSpan)): o.Lite function placeholder(name: string, span: any = jasmine.any(ParseSourceSpan)): o.PlaceholderPiece { return new o.PlaceholderPiece(name, span); } + +function humanizeSourceSpan(span: ParseSourceSpan): string { + return `"${span.toString()}" (${span.start.offset}-${span.end.offset})`; +} From 1aee8b345ac48d538c9cad62ee8c11a375828510 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 28 Oct 2020 21:35:06 +0000 Subject: [PATCH 03/11] refactor(compiler): store the `fullStart` location on `ParseSourceSpan`s (#39589) The lexer is able to skip leading trivia in the `start` location of tokens. This makes the source-span more friendly since things like elements appear to begin at the start of the opening tag, rather than at the start of any leading whitespace, which could include newlines. But some tooling requires the full source-span to be available, such as when tokenizing a text span into an Angular expression. This commit simply adds the `fullStart` location to the `ParseSourceSpan` class, and ensures that places where such spans are cloned, this property flows through too. PR Close #39589 --- .../compiler/src/compiler_facade_interface.ts | 1 + .../src/compiler_util/expression_converter.ts | 3 ++- packages/compiler/src/ml_parser/parser.ts | 21 ++++++++++----- packages/compiler/src/parse_util.ts | 26 ++++++++++++++++++- .../src/render3/view/i18n/localize_utils.ts | 3 ++- .../src/template_parser/binding_parser.ts | 4 ++- .../src/template_parser/template_parser.ts | 2 +- .../src/compiler/compiler_facade_interface.ts | 1 + 8 files changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts index 94e9a87fe920..c9741d151786 100644 --- a/packages/compiler/src/compiler_facade_interface.ts +++ b/packages/compiler/src/compiler_facade_interface.ts @@ -193,4 +193,5 @@ export interface ParseSourceSpan { start: any; end: any; details: any; + fullStart: any; } diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index 1bae07ce32ef..2727f1cba9db 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -901,7 +901,8 @@ class _AstToIrVisitor implements cdAst.AstVisitor { if (this.baseSourceSpan) { const start = this.baseSourceSpan.start.moveBy(span.start); const end = this.baseSourceSpan.start.moveBy(span.end); - return new ParseSourceSpan(start, end); + const fullStart = this.baseSourceSpan.fullStart.moveBy(span.start); + return new ParseSourceSpan(start, end, fullStart); } else { return null; } diff --git a/packages/compiler/src/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts index 3d3131989cdc..55391716162c 100644 --- a/packages/compiler/src/ml_parser/parser.ts +++ b/packages/compiler/src/ml_parser/parser.ts @@ -128,7 +128,8 @@ class _TreeBuilder { TreeError.create(null, this._peek.sourceSpan, `Invalid ICU message. Missing '}'.`)); return; } - const sourceSpan = new ParseSourceSpan(token.sourceSpan.start, this._peek.sourceSpan.end); + const sourceSpan = new ParseSourceSpan( + token.sourceSpan.start, this._peek.sourceSpan.end, token.sourceSpan.fullStart); this._addToParent(new html.Expansion( switchValue.parts[0], type.parts[0], cases, sourceSpan, switchValue.sourceSpan)); @@ -162,8 +163,10 @@ class _TreeBuilder { return null; } - const sourceSpan = new ParseSourceSpan(value.sourceSpan.start, end.sourceSpan.end); - const expSourceSpan = new ParseSourceSpan(start.sourceSpan.start, end.sourceSpan.end); + const sourceSpan = + new ParseSourceSpan(value.sourceSpan.start, end.sourceSpan.end, value.sourceSpan.fullStart); + const expSourceSpan = + new ParseSourceSpan(start.sourceSpan.start, end.sourceSpan.end, start.sourceSpan.fullStart); return new html.ExpansionCase( value.parts[0], expansionCaseParser.rootNodes, sourceSpan, value.sourceSpan, expSourceSpan); } @@ -257,8 +260,12 @@ class _TreeBuilder { selfClosing = false; } const end = this._peek.sourceSpan.start; - const span = new ParseSourceSpan(startTagToken.sourceSpan.start, end); - const el = new html.Element(fullName, attrs, [], span, span, undefined); + const span = new ParseSourceSpan( + startTagToken.sourceSpan.start, end, startTagToken.sourceSpan.fullStart); + // Create a separate `startSpan` because `span` may be modified when there is an `end` span. + const startSpan = new ParseSourceSpan( + startTagToken.sourceSpan.start, end, startTagToken.sourceSpan.fullStart); + const el = new html.Element(fullName, attrs, [], span, startSpan, undefined); this._pushElement(el); if (selfClosing) { // Elements that are self-closed have their `endSourceSpan` set to the full span, as the @@ -332,7 +339,9 @@ class _TreeBuilder { end = quoteToken.sourceSpan.end; } return new html.Attribute( - fullName, value, new ParseSourceSpan(attrName.sourceSpan.start, end), valueSpan); + fullName, value, + new ParseSourceSpan(attrName.sourceSpan.start, end, attrName.sourceSpan.fullStart), + valueSpan); } private _getParentElement(): html.Element|null { diff --git a/packages/compiler/src/parse_util.ts b/packages/compiler/src/parse_util.ts index 25ec56e7976b..190233f41328 100644 --- a/packages/compiler/src/parse_util.ts +++ b/packages/compiler/src/parse_util.ts @@ -100,8 +100,32 @@ export class ParseSourceFile { } export class ParseSourceSpan { + /** + * Create an object that holds information about spans of tokens/nodes captured during + * lexing/parsing of text. + * + * @param start + * The location of the start of the span (having skipped leading trivia). + * Skipping leading trivia makes source-spans more "user friendly", since things like HTML + * elements will appear to begin at the start of the opening tag, rather than at the start of any + * leading trivia, which could include newlines. + * + * @param end + * The location of the end of the span. + * + * @param fullStart + * The start of the token without skipping the leading trivia. + * This is used by tooling that splits tokens further, such as extracting Angular interpolations + * from text tokens. Such tooling creates new source-spans relative to the original token's + * source-span. If leading trivia characters have been skipped then the new source-spans may be + * incorrectly offset. + * + * @param details + * Additional information (such as identifier names) that should be associated with the span. + */ constructor( - public start: ParseLocation, public end: ParseLocation, public details: string|null = null) {} + public start: ParseLocation, public end: ParseLocation, + public fullStart: ParseLocation = start, public details: string|null = null) {} toString(): string { return this.start.file.content.substring(this.start.offset, this.end.offset); diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts index b57593e4d855..85b4178edf5e 100644 --- a/packages/compiler/src/render3/view/i18n/localize_utils.ts +++ b/packages/compiler/src/render3/view/i18n/localize_utils.ts @@ -90,7 +90,8 @@ function getSourceSpan(message: i18n.Message): ParseSourceSpan { const startNode = message.nodes[0]; const endNode = message.nodes[message.nodes.length - 1]; return new ParseSourceSpan( - startNode.sourceSpan.start, endNode.sourceSpan.end, startNode.sourceSpan.details); + startNode.sourceSpan.start, endNode.sourceSpan.end, startNode.sourceSpan.fullStart, + startNode.sourceSpan.details); } /** diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 1c19d69c9c8a..82c68a638cbf 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -548,5 +548,7 @@ function moveParseSourceSpan( // The difference of two absolute offsets provide the relative offset const startDiff = absoluteSpan.start - sourceSpan.start.offset; const endDiff = absoluteSpan.end - sourceSpan.end.offset; - return new ParseSourceSpan(sourceSpan.start.moveBy(startDiff), sourceSpan.end.moveBy(endDiff)); + return new ParseSourceSpan( + sourceSpan.start.moveBy(startDiff), sourceSpan.end.moveBy(endDiff), + sourceSpan.fullStart.moveBy(startDiff), sourceSpan.details); } diff --git a/packages/compiler/src/template_parser/template_parser.ts b/packages/compiler/src/template_parser/template_parser.ts index fde0f920c1f5..8444a0133408 100644 --- a/packages/compiler/src/template_parser/template_parser.ts +++ b/packages/compiler/src/template_parser/template_parser.ts @@ -566,7 +566,7 @@ class TemplateParseVisitor implements html.Visitor { const directiveAsts = directives.map((directive) => { const sourceSpan = new ParseSourceSpan( - elementSourceSpan.start, elementSourceSpan.end, + elementSourceSpan.start, elementSourceSpan.end, elementSourceSpan.fullStart, `Directive ${identifierName(directive.type)}`); if (directive.isComponent) { diff --git a/packages/core/src/compiler/compiler_facade_interface.ts b/packages/core/src/compiler/compiler_facade_interface.ts index 94e9a87fe920..c9741d151786 100644 --- a/packages/core/src/compiler/compiler_facade_interface.ts +++ b/packages/core/src/compiler/compiler_facade_interface.ts @@ -193,4 +193,5 @@ export interface ParseSourceSpan { start: any; end: any; details: any; + fullStart: any; } From ff31b431b89cf7cab22dba64c63ac8c94685cbae Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 28 Oct 2020 21:37:24 +0000 Subject: [PATCH 04/11] refactor(compiler): capture `fullStart` locations when tokenizing (#39589) This commit ensures that when leading whitespace is skipped by the tokenizer, the original start location (before skipping) is captured in the `fullStart` property of the token's source-span. PR Close #39589 --- packages/compiler/src/ml_parser/lexer.ts | 18 ++++++++++----- .../compiler/test/ml_parser/lexer_spec.ts | 22 +++++++++++++------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts index 38a82a7d23f6..d20826f9402d 100644 --- a/packages/compiler/src/ml_parser/lexer.ts +++ b/packages/compiler/src/ml_parser/lexer.ts @@ -918,19 +918,20 @@ class PlainCharacterCursor implements CharacterCursor { getSpan(start?: this, leadingTriviaCodePoints?: number[]): ParseSourceSpan { start = start || this; - let cloned = false; + let fullStart = start; if (leadingTriviaCodePoints) { while (this.diff(start) > 0 && leadingTriviaCodePoints.indexOf(start.peek()) !== -1) { - if (!cloned) { + if (fullStart === start) { start = start.clone() as this; - cloned = true; } start.advance(); } } - return new ParseSourceSpan( - new ParseLocation(start.file, start.state.offset, start.state.line, start.state.column), - new ParseLocation(this.file, this.state.offset, this.state.line, this.state.column)); + const startLocation = this.locationFromCursor(start); + const endLocation = this.locationFromCursor(this); + const fullStartLocation = + fullStart !== start ? this.locationFromCursor(fullStart) : startLocation; + return new ParseSourceSpan(startLocation, endLocation, fullStartLocation); } getChars(start: this): string { @@ -960,6 +961,11 @@ class PlainCharacterCursor implements CharacterCursor { protected updatePeek(state: CursorState): void { state.peek = state.offset >= this.end ? chars.$EOF : this.charAt(state.offset); } + + private locationFromCursor(cursor: this): ParseLocation { + return new ParseLocation( + cursor.file, cursor.state.offset, cursor.state.line, cursor.state.column); + } } class EscapedCharacterCursor extends PlainCharacterCursor { diff --git a/packages/compiler/test/ml_parser/lexer_spec.ts b/packages/compiler/test/ml_parser/lexer_spec.ts index 32895b12eabd..9ae1f053911b 100644 --- a/packages/compiler/test/ml_parser/lexer_spec.ts +++ b/packages/compiler/test/ml_parser/lexer_spec.ts @@ -54,14 +54,14 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u }); it('should skip over leading trivia for source-span start', () => { - expect(tokenizeAndHumanizeLineColumn( - '\n \t a', {leadingTriviaChars: ['\n', ' ', '\t']})) + expect( + tokenizeAndHumanizeFullStart('\n \t a', {leadingTriviaChars: ['\n', ' ', '\t']})) .toEqual([ - [lex.TokenType.TAG_OPEN_START, '0:0'], - [lex.TokenType.TAG_OPEN_END, '0:2'], - [lex.TokenType.TEXT, '1:3'], - [lex.TokenType.TAG_CLOSE, '1:4'], - [lex.TokenType.EOF, '1:8'], + [lex.TokenType.TAG_OPEN_START, '0:0', '0:0'], + [lex.TokenType.TAG_OPEN_END, '0:2', '0:2'], + [lex.TokenType.TEXT, '1:3', '0:3'], + [lex.TokenType.TAG_CLOSE, '1:4', '1:4'], + [lex.TokenType.EOF, '1:8', '1:8'], ]); }); }); @@ -1465,6 +1465,14 @@ function tokenizeAndHumanizeLineColumn(input: string, options?: lex.TokenizeOpti .tokens.map(token => [token.type, humanizeLineColumn(token.sourceSpan.start)]); } +function tokenizeAndHumanizeFullStart(input: string, options?: lex.TokenizeOptions): any[] { + return tokenizeWithoutErrors(input, options) + .tokens.map( + token => + [token.type, humanizeLineColumn(token.sourceSpan.start), + humanizeLineColumn(token.sourceSpan.fullStart)]); +} + function tokenizeAndHumanizeErrors(input: string, options?: lex.TokenizeOptions): any[] { return lex.tokenize(input, 'someUrl', getHtmlTagDefinition, options) .errors.map(e => [e.tokenType, e.msg, humanizeLineColumn(e.span.start)]); From 2b684b7e5bd238265a3139f24699e5a5a7a48794 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 29 Oct 2020 09:45:01 +0000 Subject: [PATCH 05/11] fix(compiler): skipping leading whitespace should not break placeholder source-spans (#39589) Tokenized text node may have leading whitespace skipped from their source-span. But the source-span is used to compute where there are interpolated blocks, resulting in placeholder nodes whose source-spans are offset by the amount of skipped characters. This fix uses the `fullStart` location of text source-spans for computing the source-span of placeholders, so that they are accurate. Fixes #39195 PR Close #39589 --- packages/compiler/src/i18n/i18n_parser.ts | 2 +- .../compiler/src/render3/view/template.ts | 2 +- .../compiler/test/render3/view/i18n_spec.ts | 25 +++++++++++++++++-- packages/compiler/test/render3/view/util.ts | 8 +++--- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index e8d4d7f54c80..75a068ad67fc 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -191,7 +191,7 @@ class _I18nVisitor implements html.Visitor { function getOffsetSourceSpan( sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan { - return new ParseSourceSpan(sourceSpan.start.moveBy(start), sourceSpan.start.moveBy(end)); + return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end)); } const _CUSTOM_PH_EXP = diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 8ec27e9a41cc..2d0368b4f0b3 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -52,7 +52,7 @@ const NG_PROJECT_AS_ATTR_NAME = 'ngProjectAs'; const GLOBAL_TARGET_RESOLVERS = new Map( [['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]); -const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t']; +export const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t']; // if (rf & flags) { .. } export function renderFlagCheckIfStmt( diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index 339705f1beee..b01daaaac3f7 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -18,6 +18,7 @@ import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer'; import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils'; import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta'; import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; +import {LEADING_TRIVIA_CHARS} from '../../../src/render3/view/template'; import {parseR3 as parse} from './util'; @@ -355,7 +356,7 @@ describe('serializeI18nMessageForGetMsg', () => { describe('serializeI18nMessageForLocalize', () => { const serialize = (input: string) => { - const tree = parse(`
${input}
`); + const tree = parse(`
${input}
`, {leadingTriviaChars: LEADING_TRIVIA_CHARS}); const root = tree.nodes[0] as t.Element; return serializeI18nMessageForLocalize(root.i18n as i18n.Message); }; @@ -446,7 +447,7 @@ describe('serializeI18nMessageForLocalize', () => { expect(messageParts[3].text).toEqual(''); expect(messageParts[3].sourceSpan.toString()).toEqual(''); expect(messageParts[4].text).toEqual(' D'); - expect(messageParts[4].sourceSpan.toString()).toEqual(' D'); + expect(messageParts[4].sourceSpan.toString()).toEqual('D'); expect(placeHolders[0].text).toEqual('START_TAG_SPAN'); expect(placeHolders[0].sourceSpan.toString()).toEqual(''); @@ -478,6 +479,26 @@ describe('serializeI18nMessageForLocalize', () => { expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"" (22-26)'); }); + it('should create the correct placeholder source-spans when there is skipped leading whitespace', + () => { + const {messageParts, placeHolders} = serialize(' {{value}}'); + expect(messageParts[0].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)'); + expect(messageParts[1].text).toEqual(' '); + expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('" " (13-16)'); + expect(messageParts[2].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (25-25)'); + expect(messageParts[3].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (29-29)'); + + expect(placeHolders[0].text).toEqual('START_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('" " (10-16)'); + expect(placeHolders[1].text).toEqual('INTERPOLATION'); + expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (16-25)'); + expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"" (25-29)'); + }); + it('should serialize simple ICU for `$localize()`', () => { expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({ messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')], diff --git a/packages/compiler/test/render3/view/util.ts b/packages/compiler/test/render3/view/util.ts index af3ed25c1182..e974e0e155cf 100644 --- a/packages/compiler/test/render3/view/util.ts +++ b/packages/compiler/test/render3/view/util.ts @@ -78,11 +78,13 @@ export function toStringExpression(expr: e.AST): string { // Parse an html string to IVY specific info export function parseR3( - input: string, options: {preserveWhitespaces?: boolean} = {}): Render3ParseResult { + input: string, options: {preserveWhitespaces?: boolean, leadingTriviaChars?: string[]} = {}): + Render3ParseResult { const htmlParser = new HtmlParser(); - const parseResult = - htmlParser.parse(input, 'path:://to/template', {tokenizeExpansionForms: true}); + const parseResult = htmlParser.parse( + input, 'path:://to/template', + {tokenizeExpansionForms: true, leadingTriviaChars: options.leadingTriviaChars}); if (parseResult.errors.length > 0) { const msg = parseResult.errors.map(e => e.toString()).join('\n'); From 7bd01334225b3acc39e16bca2a5f4aede4b920b6 Mon Sep 17 00:00:00 2001 From: Athur Ming Date: Fri, 6 Nov 2020 14:57:34 +0800 Subject: [PATCH 06/11] docs: typo fix for 'Intall' (#39585) 'Intall' should be 'Install' PR Close #39585 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cef5f16b88fb..58aac4dc2c05 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Get started with Angular, learn the fundamentals and explore advanced topics on ### Setting Up a Project -Intall the Angular CLI globally: +Install the Angular CLI globally: ``` npm install -g @angular/cli From 1c6cf8a8c22e7b9b46157c8ec008c491e0ddbb1e Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 5 Nov 2020 11:01:45 +0000 Subject: [PATCH 07/11] fix(compiler-cli): do not drop non-Angular decorators when downleveling (#39577) There is a compiler transform that downlevels Angular class decorators to static properties so that metadata is available for JIT compilation. The transform was supposed to ignore non-Angular decorators but it was actually completely dropping decorators that did not conform to a very specific syntactic shape (i.e. the decorator was a simple identifier, or a namespaced identifier). This commit ensures that all non-Angular decorators are kepts as-is even if they are built using a syntax that the Angular compiler does not understand. Fixes #39574 PR Close #39577 --- .../downlevel_decorators_transform.ts | 19 ++++++++++++------- .../downlevel_decorators_transform_spec.ts | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts index 97a0465b0a13..5c4cc4627c3b 100644 --- a/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts +++ b/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts @@ -538,12 +538,17 @@ export function getDownlevelDecoratorsTransform( } newMembers.push(ts.visitEachChild(member, decoratorDownlevelVisitor, context)); } - const decorators = host.getDecoratorsOfDeclaration(classDecl) || []; + + // The `ReflectionHost.getDecoratorsOfDeclaration()` method will not return certain kinds of + // decorators that will never be Angular decorators. So we cannot rely on it to capture all + // the decorators that should be kept. Instead we start off with a set of the raw decorators + // on the class, and only remove the ones that have been identified for downleveling. + const decoratorsToKeep = new Set(classDecl.decorators); + const possibleAngularDecorators = host.getDecoratorsOfDeclaration(classDecl) || []; let hasAngularDecorator = false; const decoratorsToLower = []; - const decoratorsToKeep: ts.Decorator[] = []; - for (const decorator of decorators) { + for (const decorator of possibleAngularDecorators) { // We only deal with concrete nodes in TypeScript sources, so we don't // need to handle synthetically created decorators. const decoratorNode = decorator.node! as ts.Decorator; @@ -557,8 +562,7 @@ export function getDownlevelDecoratorsTransform( if (isNgDecorator && !skipClassDecorators) { decoratorsToLower.push(extractMetadataFromSingleDecorator(decoratorNode, diagnostics)); - } else { - decoratorsToKeep.push(decoratorNode); + decoratorsToKeep.delete(decoratorNode); } } @@ -581,8 +585,9 @@ export function getDownlevelDecoratorsTransform( ts.createNodeArray(newMembers, classDecl.members.hasTrailingComma), classDecl.members); return ts.updateClassDeclaration( - classDecl, decoratorsToKeep.length ? decoratorsToKeep : undefined, classDecl.modifiers, - classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, members); + classDecl, decoratorsToKeep.size ? Array.from(decoratorsToKeep) : undefined, + classDecl.modifiers, classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, + members); } /** diff --git a/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts index daf5d946a051..28fc1708a9ce 100644 --- a/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts +++ b/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts @@ -189,6 +189,21 @@ describe('downlevel decorator transform', () => { expect(output).not.toContain('MyClass.decorators'); }); + it('should not downlevel non-Angular class decorators generated by a builder', () => { + const {output} = transform(` + @DecoratorBuilder().customClassDecorator + export class MyClass {} + `); + + expect(diagnostics.length).toBe(0); + expect(output).toContain(dedent` + MyClass = tslib_1.__decorate([ + DecoratorBuilder().customClassDecorator + ], MyClass); + `); + expect(output).not.toContain('MyClass.decorators'); + }); + it('should downlevel Angular-decorated class member', () => { const {output} = transform(` import {Input} from '@angular/core'; From 861e4fa627e34a1a83afee4e0f63015ff25ddbaf Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 30 Oct 2020 14:37:02 -0700 Subject: [PATCH 08/11] fix(compiler-cli): avoid duplicate diagnostics about unknown pipes (#39517) TCB generation occasionally transforms binding expressions twice, which can result in a `BindingPipe` operation being `resolve()`'d multiple times. When the pipe does not exist, this caused multiple OOB diagnostics to be recorded about the missing pipe. This commit fixes the problem by making the OOB recorder track which pipe expressions have had diagnostics produced already, and only producing them once per expression. PR Close #39517 --- .../src/ngtsc/typecheck/src/oob.ts | 11 ++++++++ .../ngtsc/typecheck/test/diagnostics_spec.ts | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index 2b7cbd02e534..75170a918a75 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -73,6 +73,12 @@ export interface OutOfBandDiagnosticRecorder { export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder { private _diagnostics: TemplateDiagnostic[] = []; + /** + * Tracks which `BindingPipe` nodes have already been recorded as invalid, so only one diagnostic + * is ever produced per node. + */ + private recordedPipes = new Set(); + constructor(private resolver: TemplateSourceResolver) {} get diagnostics(): ReadonlyArray { @@ -90,6 +96,10 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor } missingPipe(templateId: TemplateId, ast: BindingPipe): void { + if (this.recordedPipes.has(ast)) { + return; + } + const mapping = this.resolver.getSourceMapping(templateId); const errorMsg = `No pipe found with name '${ast.name}'.`; @@ -101,6 +111,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor this._diagnostics.push(makeTemplateDiagnostic( templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg)); + this.recordedPipes.add(ast); } illegalAssignmentToTemplateVar( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 5838f2fc8e1a..e2adb50f5800 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -195,6 +195,32 @@ runInEachFileSystem(() => { ]); }); + it('does not repeat diagnostics for missing pipes in directive inputs', () => { + // The directive here is structured so that a type constructor is used, which resuts in each + // input binding being processed twice. This results in the 'uppercase' pipe being resolved + // twice, and since it doesn't exist this operation will fail. The test is here to verify that + // failing to resolve the pipe twice only produces a single diagnostic (no duplicates). + const messages = diagnose( + '
', ` + class Dir { + dirOf: T; + } + + class TestComponent { + name: string; + }`, + [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'dirOf': 'dirOf'}, + isGeneric: true, + }]); + + expect(messages.length).toBe(1); + expect(messages[0]).toContain(`No pipe found with name 'uppercase'.`); + }); + it('does not repeat diagnostics for errors within LHS of safe-navigation operator', () => { const messages = diagnose(`{{ personn?.name }} {{ personn?.getName() }}`, ` class TestComponent { From 1357d84ca982cc940f7a848102a372b602dd974d Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 5 Nov 2020 18:26:14 +0200 Subject: [PATCH 09/11] build(docs-infra): print the git commit when deploying to Firebase (#39596) The commit updates the AIO deployment script to also print the commit SHA. This makes it easier to check whether a version has been successfully deployed, by comparing the commit SHA from the CI job with the SHA in the version string in the footer of the AIO app. PR Close #39596 --- aio/scripts/deploy-to-firebase.js | 1 + aio/scripts/deploy-to-firebase.spec.js | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/aio/scripts/deploy-to-firebase.js b/aio/scripts/deploy-to-firebase.js index a381a7a592f4..af327f0a31c1 100644 --- a/aio/scripts/deploy-to-firebase.js +++ b/aio/scripts/deploy-to-firebase.js @@ -31,6 +31,7 @@ if (require.main === module) { } else { console.log( `Git branch : ${inputVars.currentBranch}\n` + + `Git commit : ${inputVars.currentCommit}\n` + `Build/deploy mode : ${deploymentInfo.deployEnv}\n` + `Firebase project : ${deploymentInfo.projectId}\n` + `Firebase site : ${deploymentInfo.siteId}\n` + diff --git a/aio/scripts/deploy-to-firebase.spec.js b/aio/scripts/deploy-to-firebase.spec.js index 02a09a3aed13..696e7c66521a 100644 --- a/aio/scripts/deploy-to-firebase.spec.js +++ b/aio/scripts/deploy-to-firebase.spec.js @@ -262,17 +262,19 @@ describe('deploy-to-firebase:', () => { }); it('integration - should run the main script without error', () => { + const commit = getLatestCommit('master'); const cmd = `"${process.execPath}" "${__dirname}/deploy-to-firebase" --dry-run`; const env = { CI_REPO_OWNER: 'angular', CI_REPO_NAME: 'angular', CI_PULL_REQUEST: 'false', CI_BRANCH: 'master', - CI_COMMIT: getLatestCommit('master') + CI_COMMIT: commit, }; const result = execSync(cmd, {encoding: 'utf8', env}).trim(); expect(result).toBe( 'Git branch : master\n' + + `Git commit : ${commit}\n` + 'Build/deploy mode : next\n' + 'Firebase project : angular-io\n' + 'Firebase site : next-angular-io-site\n' + From ae71b760890a5f2c5fdd41e9783d90dbec1f7ee5 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 7 Nov 2020 15:11:47 +0200 Subject: [PATCH 10/11] ci: log commands output when deploying angular.io to Firebase (#39596) In #39470, the `deploy-to-firebase.sh` script (used to deploy AIO to Firebase when building an upstream branch), was replaced by an equivalent JS script. In this new `deploy-to-firebase.js` script, we were overly aggressive with suppressing command output, which made it hard to investigate failures ([example failing CI job][1]). This commit updates the `deploy-to-firebase.js` script to capture command output as usual in the CI job logs. This makes the output similar to the one generated by the old [deploy-to-firebase.sh][2] script ([example CI logs][3]). One concern with capturing command output is having the value of a secret environment variables leaked in the logs. This is not the case here, since: 1. The secret env vars are not printed from the commands that use them. 2. CircleCI will [mask the values of secret env vars][4] in the output. As an extra precaution (although not strictly necessary), we run `yarn` with the `--silent` option, which avoid echoing the executed yarn commands. [1]: https://circleci.com/gh/angular/angular/849310 [2]: https://github.com/angular/angular/blob/3b0b7d22109c79b4dceb/aio/scripts/deploy-to-firebase.sh [3]: https://circleci.com/gh/angular/angular/848109 [4]: https://circleci.com/docs/2.0/env-vars/#secrets-masking PR Close #39596 --- aio/scripts/deploy-to-firebase.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/aio/scripts/deploy-to-firebase.js b/aio/scripts/deploy-to-firebase.js index af327f0a31c1..b2975f9a3357 100644 --- a/aio/scripts/deploy-to-firebase.js +++ b/aio/scripts/deploy-to-firebase.js @@ -4,7 +4,7 @@ // 'use strict'; -const {cd, cp, exec: _exec, set} = require('shelljs'); +const {cd, cp, exec, set} = require('shelljs'); set('-e'); @@ -187,13 +187,8 @@ function deploy( yarn(`test-pwa-score "${deployedUrl}" "${minPwaScore}"`); } -function exec(cmd, opts) { - // Using `silent: true` to ensure no secret env variables are printed. - return _exec(cmd, {silent: true, ...opts}).trim(); -} - function getRemoteRefs(refOrPattern, remote = NG_REMOTE_URL) { - return exec(`git ls-remote ${remote} ${refOrPattern}`).split('\n'); + return exec(`git ls-remote ${remote} ${refOrPattern}`, {silent: true}).trim().split('\n'); } function getLatestCommit(branchName, remote = undefined) { @@ -206,5 +201,10 @@ function skipDeployment(reason) { function yarn(cmd) { // Using `--silent` to ensure no secret env variables are printed. + // + // NOTE: + // This is not strictly necessary, since CircleCI will mask secret environment variables in the + // output (see https://circleci.com/docs/2.0/env-vars/#secrets-masking), but is an extra + // precaution. return exec(`yarn --silent ${cmd}`); } From 0786c59835eb7f0fe374a5efe5a1f5b3b5cf76c7 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 9 Nov 2020 12:46:39 -0800 Subject: [PATCH 11/11] release: cut the v10.2.3 release --- CHANGELOG.md | 13 +++++++++++++ package.json | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e92925ea539..423c6511a020 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ + +## 10.2.3 (2020-11-09) + + +### Bug Fixes + +* **compiler:** ensure that i18n message-parts have the correct source-span ([#39589](https://github.com/angular/angular/issues/39589)) ([e67a331](https://github.com/angular/angular/commit/e67a331)) +* **compiler:** skipping leading whitespace should not break placeholder source-spans ([#39589](https://github.com/angular/angular/issues/39589)) ([2b684b7](https://github.com/angular/angular/commit/2b684b7)), closes [#39195](https://github.com/angular/angular/issues/39195) +* **compiler-cli:** avoid duplicate diagnostics about unknown pipes ([#39517](https://github.com/angular/angular/issues/39517)) ([861e4fa](https://github.com/angular/angular/commit/861e4fa)) +* **compiler-cli:** do not drop non-Angular decorators when downleveling ([#39577](https://github.com/angular/angular/issues/39577)) ([1c6cf8a](https://github.com/angular/angular/commit/1c6cf8a)), closes [#39574](https://github.com/angular/angular/issues/39574) + + + ## 10.2.2 (2020-11-04) diff --git a/package.json b/package.json index 143b6c0b55d0..3eed76e9f501 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "10.2.2", + "version": "10.2.3", "private": true, "description": "Angular - a web framework for modern web apps", "homepage": "https://github.com/angular/angular",