From 54af220107c9db6c91ce2f11336c2b00bf8e98bc Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Sun, 15 Dec 2019 21:05:24 -0800 Subject: [PATCH] refactor(ivy): create better styling parsing API (#34418) Parsing styling is now simplified to be used like so: ``` for (let i = parseStyle(text); i <= 0; i = parseStyleNext(text, i)) { const key = getLastParsedKey(); const value = getLastParsedValue(); ... } ``` This change makes it easier to invoke the parser from other locations in the system without paying the cost of creating and iterating over `Map` of styles. PR Closes #34418 --- .../core/src/render3/styling/class_differ.ts | 15 +- .../core/src/render3/styling/style_differ.ts | 82 ++------ .../src/render3/styling/styling_parser.ts | 178 +++++++++++++++++- .../render3/styling_next/style_differ_spec.ts | 7 +- 4 files changed, 204 insertions(+), 78 deletions(-) diff --git a/packages/core/src/render3/styling/class_differ.ts b/packages/core/src/render3/styling/class_differ.ts index b106cc7dec..09375e08b1 100644 --- a/packages/core/src/render3/styling/class_differ.ts +++ b/packages/core/src/render3/styling/class_differ.ts @@ -7,7 +7,9 @@ */ import {CharCode} from '../../util/char_code'; -import {consumeClassToken, consumeWhitespace} from './styling_parser'; + +import {consumeWhitespace, getLastParsedKey, parseClassName, parseClassNameNext} from './styling_parser'; + /** * Computes the diff between two class-list strings. @@ -50,15 +52,8 @@ export function computeClassChanges(oldValue: string, newValue: string): Map, isNewValue: boolean): void { - const end = text.length; - let index = 0; - while (index < end) { - index = consumeWhitespace(text, index, end); - const tokenEnd = consumeClassToken(text, index, end); - if (tokenEnd !== index) { - processClassToken(changes, text.substring(index, tokenEnd), isNewValue); - } - index = tokenEnd; + for (let i = parseClassName(text); i >= 0; i = parseClassNameNext(text, i)) { + processClassToken(changes, getLastParsedKey(text), isNewValue); } } diff --git a/packages/core/src/render3/styling/style_differ.ts b/packages/core/src/render3/styling/style_differ.ts index 714d5ca1a7..80459b7ac4 100644 --- a/packages/core/src/render3/styling/style_differ.ts +++ b/packages/core/src/render3/styling/style_differ.ts @@ -6,10 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CharCode} from '../../util/char_code'; -import {consumeSeparator, consumeStyleKey, consumeStyleValue, consumeWhitespace} from './styling_parser'; - - +import {getLastParsedKey, getLastParsedValue, getLastParsedValueEnd, parseStyle, parseStyleNext, resetParserState} from './styling_parser'; /** * Stores changes to Style values. @@ -60,24 +57,8 @@ export function computeStyleChanges(oldValue: string, newValue: string): StyleCh * @param isNewValue `true` if parsing new value (effects how values get added to `changes`) */ export function parseKeyValue(text: string, changes: StyleChangesMap, isNewValue: boolean): void { - const end = text.length; - let start = 0; - while (start < end) { - const keyStart = consumeWhitespace(text, start, end); - const keyEnd = consumeStyleKey(text, keyStart, end); - if (keyEnd === keyStart) { - // we reached an end so just quit - break; - } - const valueStart = consumeSeparator(text, keyEnd, end, CharCode.COLON); - const valueEnd = consumeStyleValue(text, valueStart, end); - if (ngDevMode && valueStart === valueEnd) { - throw malformedStyleError(text, valueStart); - } - start = consumeSeparator(text, valueEnd, end, CharCode.SEMI_COLON); - const key = text.substring(keyStart, keyEnd); - const value = text.substring(valueStart, valueEnd); - processStyleKeyValue(changes, key, value, isNewValue); + for (let i = parseStyle(text); i >= 0; i = parseStyleNext(text, i)) { + processStyleKeyValue(changes, getLastParsedKey(text), getLastParsedValue(text), isNewValue); } } @@ -122,51 +103,26 @@ function styleKeyValue(oldValue: string | null, newValue: string | null) { * @returns a new style text which does not have `styleToRemove` (and its value) */ export function removeStyle(cssText: string, styleToRemove: string): string { - let start = 0; - let end = cssText.length; + if (cssText.indexOf(styleToRemove) === -1) { + // happy case where we don't need to invoke parser. + return cssText; + } let lastValueEnd = 0; - while (start < end) { - const possibleKeyIndex = cssText.indexOf(styleToRemove, start); - if (possibleKeyIndex === -1) { - // we did not find anything, so just bail. - break; - } - while (start < possibleKeyIndex + 1) { - const keyStart = consumeWhitespace(cssText, start, end); - const keyEnd = consumeStyleKey(cssText, keyStart, end); - if (keyEnd === keyStart) { - // we reached the end - return cssText; - } - const valueStart = consumeSeparator(cssText, keyEnd, end, CharCode.COLON); - const valueEnd = consumeStyleValue(cssText, valueStart, end); - if (ngDevMode && valueStart === valueEnd) { - throw malformedStyleError(cssText, valueStart); - } - const valueEndSep = consumeSeparator(cssText, valueEnd, end, CharCode.SEMI_COLON); - if (keyStart == possibleKeyIndex && keyEnd === possibleKeyIndex + styleToRemove.length) { - if (valueEndSep == end) { - // This is a special case when we are the last key in a list, we then chop off the - // trailing separator as well. - cssText = cssText.substring(0, lastValueEnd); - } else { - cssText = cssText.substring(0, keyStart) + cssText.substring(valueEndSep, end); - } - end = cssText.length; - start = keyStart; - break; // rescan. + for (let i = parseStyle(cssText); i >= 0; i = parseStyleNext(cssText, i)) { + const key = getLastParsedKey(cssText); + if (key === styleToRemove) { + if (lastValueEnd === 0) { + cssText = cssText.substring(i); + i = 0; + } else if (i === cssText.length) { + return cssText.substring(0, lastValueEnd); } else { - // This was not the item we are looking for, keep going. - start = valueEndSep; + cssText = cssText.substring(0, lastValueEnd) + '; ' + cssText.substring(i); + i = lastValueEnd + 2; // 2 is for '; '.length(so that we skip the separator) } - lastValueEnd = valueEnd; + resetParserState(cssText); } + lastValueEnd = getLastParsedValueEnd(); } return cssText; } - -function malformedStyleError(text: string, index: number) { - return new Error( - `Malformed style at location ${index} in string '` + text.substring(0, index) + '[>>' + - text.substring(index, index + 1) + '<<]' + text.substr(index + 1) + '\'.'); -} \ No newline at end of file diff --git a/packages/core/src/render3/styling/styling_parser.ts b/packages/core/src/render3/styling/styling_parser.ts index 64f32c7843..be097b9d8f 100644 --- a/packages/core/src/render3/styling/styling_parser.ts +++ b/packages/core/src/render3/styling/styling_parser.ts @@ -8,6 +8,174 @@ import {CharCode} from '../../util/char_code'; +/** + * Stores the locations of key/value indexes while parsing styling. + * + * In case of `cssText` parsing the indexes are like so: + * ``` + * "key1: value1; key2: value2; key3: value3" + * ^ ^ ^ ^ ^ + * | | | | +-- textEnd + * | | | +---------------- valueEnd + * | | +---------------------- value + * | +------------------------ keyEnd + * +---------------------------- key + * ``` + * + * In case of `className` parsing the indexes are like so: + * ``` + * "key1 key2 key3" + * ^ ^ ^ + * | | +-- textEnd + * | +------------------------ keyEnd + * +---------------------------- key + * ``` + * NOTE: `value` and `valueEnd` are used only for styles, not classes. + */ +interface ParserState { + textEnd: number; + key: number; + keyEnd: number; + value: number; + valueEnd: number; +} +// Global state of the parser. (This makes parser non-reentrant, but that is not an issue) +const parserState: ParserState = { + textEnd: 0, + key: 0, + keyEnd: 0, + value: 0, + valueEnd: 0, +}; + +/** + * Retrieves the last parsed `key` of style. + * @param text the text to substring the key from. + */ +export function getLastParsedKey(text: string): string { + return text.substring(parserState.key, parserState.keyEnd); +} + +/** + * Retrieves the last parsed `value` of style. + * @param text the text to substring the key from. + */ +export function getLastParsedValue(text: string): string { + return text.substring(parserState.value, parserState.valueEnd); +} + +/** + * Initializes `className` string for parsing and parses the first token. + * + * This function is intended to be used in this format: + * ``` + * for (let i = parseClassName(text); i >= 0; i = parseClassNameNext(text, i))) { + * const key = getLastParsedKey(); + * ... + * } + * ``` + * @param text `className` to parse + * @returns index where the next invocation of `parseClassNameNext` should resume. + */ +export function parseClassName(text: string): number { + resetParserState(text); + return parseClassNameNext(text, consumeWhitespace(text, 0, parserState.textEnd)); +} + +/** + * Parses next `className` token. + * + * This function is intended to be used in this format: + * ``` + * for (let i = parseClassName(text); i >= 0; i = parseClassNameNext(text, i))) { + * const key = getLastParsedKey(); + * ... + * } + * ``` + * + * @param text `className` to parse + * @param index where the parsing should resume. + * @returns index where the next invocation of `parseClassNameNext` should resume. + */ +export function parseClassNameNext(text: string, index: number): number { + const end = parserState.textEnd; + if (end === index) { + return -1; + } + index = parserState.keyEnd = consumeClassToken(text, parserState.key = index, end); + return consumeWhitespace(text, index, end); +} + +/** + * Initializes `cssText` string for parsing and parses the first key/values. + * + * This function is intended to be used in this format: + * ``` + * for (let i = parseStyle(text); i >= 0; i = parseStyleNext(text, i))) { + * const key = getLastParsedKey(); + * const value = getLastParsedValue(); + * ... + * } + * ``` + * @param text `cssText` to parse + * @returns index where the next invocation of `parseStyleNext` should resume. + */ +export function parseStyle(text: string): number { + resetParserState(text); + return parseStyleNext(text, consumeWhitespace(text, 0, parserState.textEnd)); +} + +/** + * Parses the next `cssText` key/values. + * + * This function is intended to be used in this format: + * ``` + * for (let i = parseStyle(text); i >= 0; i = parseStyleNext(text, i))) { + * const key = getLastParsedKey(); + * const value = getLastParsedValue(); + * ... + * } + * + * @param text `cssText` to parse + * @param index where the parsing should resume. + * @returns index where the next invocation of `parseStyleNext` should resume. + */ +export function parseStyleNext(text: string, startIndex: number): number { + const end = parserState.textEnd; + if (end === startIndex) { + // we reached an end so just quit + return -1; + } + let index = parserState.keyEnd = consumeStyleKey(text, parserState.key = startIndex, end); + index = parserState.value = consumeSeparatorWithWhitespace(text, index, end, CharCode.COLON); + index = parserState.valueEnd = consumeStyleValue(text, index, end); + if (ngDevMode && parserState.value === parserState.valueEnd) { + throw malformedStyleError(text, index); + } + return consumeSeparatorWithWhitespace(text, index, end, CharCode.SEMI_COLON); +} + +/** + * Reset the global state of the styling parser. + * @param text The styling text to parse. + */ +export function resetParserState(text: string): void { + parserState.key = 0; + parserState.keyEnd = 0; + parserState.value = 0; + parserState.valueEnd = 0; + parserState.textEnd = text.length; +} + +/** + * Retrieves tha `valueEnd` from the parser global state. + * + * See: `ParserState`. + */ +export function getLastParsedValueEnd(): number { + return parserState.valueEnd; +} + /** * Returns index of next non-whitespace character. * @@ -65,7 +233,7 @@ export function consumeStyleKey(text: string, startIndex: number, endIndex: numb * @param endIndex Ending index of character where the scan should end. * @returns Index after separator and surrounding whitespace. */ -export function consumeSeparator( +export function consumeSeparatorWithWhitespace( text: string, startIndex: number, endIndex: number, separator: number): number { startIndex = consumeWhitespace(text, startIndex, endIndex); if (startIndex < endIndex) { @@ -150,4 +318,10 @@ function expectingError(text: string, expecting: string, index: number) { return new Error( `Expecting '${expecting}' at location ${index} in string '` + text.substring(0, index) + '[>>' + text.substring(index, index + 1) + '<<]' + text.substr(index + 1) + '\'.'); -} \ No newline at end of file +} + +function malformedStyleError(text: string, index: number) { + return new Error( + `Malformed style at location ${index} in string '` + text.substring(0, index) + '[>>' + + text.substring(index, index + 1) + '<<]' + text.substr(index + 1) + '\'.'); +} diff --git a/packages/core/test/render3/styling_next/style_differ_spec.ts b/packages/core/test/render3/styling_next/style_differ_spec.ts index e161434f4c..6d7d51e995 100644 --- a/packages/core/test/render3/styling_next/style_differ_spec.ts +++ b/packages/core/test/render3/styling_next/style_differ_spec.ts @@ -7,7 +7,7 @@ */ import {StyleChangesMap, parseKeyValue, removeStyle} from '@angular/core/src/render3/styling/style_differ'; -import {consumeSeparator, consumeStyleValue} from '@angular/core/src/render3/styling/styling_parser'; +import {consumeSeparatorWithWhitespace, consumeStyleValue} from '@angular/core/src/render3/styling/styling_parser'; import {CharCode} from '@angular/core/src/util/char_code'; import {sortedForEach} from './class_differ_spec'; @@ -92,6 +92,7 @@ describe('style differ', () => { it('should remove some of the style', () => { expect(removeStyle('a: a; foo: bar; b: b', 'foo')).toEqual('a: a; b: b'); + expect(removeStyle('a: a; foo: bar; b: b', 'foo')).toEqual('a: a; b: b'); expect(removeStyle('a: a; foo: bar; b: b; foo: bar; c: c', 'foo')) .toEqual('a: a; b: b; c: c'); }); @@ -113,9 +114,9 @@ function expectParseValue( text: string) { let stopIndex = text.indexOf('🛑'); if (stopIndex < 0) stopIndex = text.length; - const valueStart = consumeSeparator(text, 0, text.length, CharCode.COLON); + const valueStart = consumeSeparatorWithWhitespace(text, 0, text.length, CharCode.COLON); const valueEnd = consumeStyleValue(text, valueStart, text.length); - const valueSep = consumeSeparator(text, valueEnd, text.length, CharCode.SEMI_COLON); + const valueSep = consumeSeparatorWithWhitespace(text, valueEnd, text.length, CharCode.SEMI_COLON); expect(valueSep).toBe(stopIndex); return expect(text.substring(valueStart, valueEnd)); }