diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 0086b1ca34..9945a54f07 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 13604, + "main": 13415, "polyfills": 45340 } } diff --git a/packages/core/src/render3/instructions/advance.ts b/packages/core/src/render3/instructions/advance.ts index 86707b3781..307d9ece05 100644 --- a/packages/core/src/render3/instructions/advance.ts +++ b/packages/core/src/render3/instructions/advance.ts @@ -9,7 +9,6 @@ import {assertDataInRange, assertGreaterThan} from '../../util/assert'; import {executeCheckHooks, executeInitAndCheckHooks} from '../hooks'; import {FLAGS, HEADER_OFFSET, InitPhaseState, LView, LViewFlags, TVIEW} from '../interfaces/view'; import {ActiveElementFlags, executeElementExitFn, getCheckNoChangesMode, getLView, getSelectedIndex, hasActiveElementFlag, setSelectedIndex} from '../state'; -import {resetStylingState} from '../styling_next/state'; @@ -76,10 +75,6 @@ export function selectIndexInternal(lView: LView, index: number, checkNoChangesM } } - if (hasActiveElementFlag(ActiveElementFlags.ResetStylesOnExit)) { - resetStylingState(); - } - // We must set the selected index *after* running the hooks, because hooks may have side-effects // that cause other template functions to run, thus updating the selected index, which is global // state. If we run `setSelectedIndex` *before* we run the hooks, in some cases the selected index diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index d3e7fd1d80..74fd5e88da 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -32,7 +32,6 @@ import {assertNodeOfPossibleTypes} from '../node_assert'; import {isNodeMatchingSelectorList} from '../node_selector_matcher'; import {ActiveElementFlags, executeElementExitFn, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, hasActiveElementFlag, incrementActiveDirectiveId, namespaceHTMLInternal, selectView, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; import {renderStylingMap} from '../styling_next/bindings'; -import {resetStylingState} from '../styling_next/state'; import {NO_CHANGE} from '../tokens'; import {ANIMATION_PROP_PREFIX, isAnimationProp} from '../util/attrs_utils'; import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils'; @@ -505,9 +504,6 @@ function executeTemplate( if (hasActiveElementFlag(ActiveElementFlags.RunExitFn)) { executeElementExitFn(); } - if (hasActiveElementFlag(ActiveElementFlags.ResetStylesOnExit)) { - resetStylingState(); - } setSelectedIndex(prevSelectedIndex); } } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 0699a1e42a..ce7d5d2a67 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -12,8 +12,7 @@ import {assertDefined} from '../util/assert'; import {assertLViewOrUndefined} from './assert'; import {ComponentDef, DirectiveDef} from './interfaces/definition'; import {TElementNode, TNode, TViewNode} from './interfaces/node'; -import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState, TVIEW} from './interfaces/view'; -import {resetStylingState} from './styling_next/state'; +import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState} from './interfaces/view'; /** @@ -144,8 +143,7 @@ let activeDirectiveId = 0; export const enum ActiveElementFlags { Initial = 0b00, RunExitFn = 0b01, - ResetStylesOnExit = 0b10, - Size = 2, + Size = 1, } /** @@ -174,9 +172,6 @@ export function setActiveHostElement(elementIndex: number | null = null) { if (hasActiveElementFlag(ActiveElementFlags.RunExitFn)) { executeElementExitFn(); } - if (hasActiveElementFlag(ActiveElementFlags.ResetStylesOnExit)) { - resetStylingState(); - } setSelectedIndex(elementIndex === null ? -1 : elementIndex); activeDirectiveId = 0; } @@ -390,6 +385,10 @@ export function setCurrentQueryIndex(value: number): void { * @returns the previously active lView; */ export function selectView(newView: LView, hostTNode: TElementNode | TViewNode | null): LView { + if (hasActiveElementFlag(ActiveElementFlags.RunExitFn)) { + executeElementExitFn(); + } + ngDevMode && assertLViewOrUndefined(newView); const oldView = lView; diff --git a/packages/core/src/render3/styling_next/bindings.ts b/packages/core/src/render3/styling_next/bindings.ts index cc40299b64..1a216b826c 100644 --- a/packages/core/src/render3/styling_next/bindings.ts +++ b/packages/core/src/render3/styling_next/bindings.ts @@ -405,18 +405,22 @@ export function flushStyling( if (!isContextLocked(stylesContext, hostBindingsMode)) { lockAndFinalizeContext(stylesContext, hostBindingsMode); } - applyStylingViaContext( - stylesContext, renderer, element, data, state.stylesBitMask, setStyle, styleSanitizer, - hostBindingsMode); + if (state.stylesBitMask !== 0) { + applyStylingViaContext( + stylesContext, renderer, element, data, state.stylesBitMask, setStyle, styleSanitizer, + hostBindingsMode); + } } if (classesContext) { if (!isContextLocked(classesContext, hostBindingsMode)) { lockAndFinalizeContext(classesContext, hostBindingsMode); } - applyStylingViaContext( - classesContext, renderer, element, data, state.classesBitMask, setClass, null, - hostBindingsMode); + if (state.classesBitMask !== 0) { + applyStylingViaContext( + classesContext, renderer, element, data, state.classesBitMask, setClass, null, + hostBindingsMode); + } } resetStylingState(); diff --git a/packages/core/src/render3/styling_next/instructions.ts b/packages/core/src/render3/styling_next/instructions.ts index f967d27e76..bf4688d468 100644 --- a/packages/core/src/render3/styling_next/instructions.ts +++ b/packages/core/src/render3/styling_next/instructions.ts @@ -11,7 +11,7 @@ import {setInputsForProperty} from '../instructions/shared'; import {AttributeMarker, TAttributes, TNode, TNodeType} from '../interfaces/node'; import {RElement} from '../interfaces/renderer'; import {BINDING_INDEX, LView, RENDERER} from '../interfaces/view'; -import {ActiveElementFlags, getActiveDirectiveId, getCurrentStyleSanitizer, getLView, getSelectedIndex, setActiveElementFlag, setCurrentStyleSanitizer, setElementExitFn} from '../state'; +import {getActiveDirectiveId, getCurrentStyleSanitizer, getLView, getSelectedIndex, setCurrentStyleSanitizer, setElementExitFn} from '../state'; import {NO_CHANGE} from '../tokens'; import {renderStringify} from '../util/misc_utils'; import {getNativeByTNode, getTNode} from '../util/view_utils'; @@ -186,10 +186,7 @@ function stylingProp( value as string | SafeValue | null, sanitizer); } - if (updated) { - setElementExitFn(applyStyling); - } - markStylingStateAsDirty(); + setElementExitFn(stylingApply); } return updated; @@ -331,6 +328,9 @@ function _stylingMap( renderer, context, native, lView, bindingIndex, stylingMapArr as StylingMapArray, isClassBased ? setClass : setStyle, sanitizer, valueHasChanged); } else { + updated = valueHasChanged; + activateStylingMapFeature(); + // Context Resolution (or first update) Case: save the map value // and defer to the context to flush and apply the style/class binding // value to the element. @@ -344,15 +344,9 @@ function _stylingMap( valueHasChanged); } - if (valueHasChanged) { - updated = true; - setElementExitFn(applyStyling); - } - - activateStylingMapFeature(); + setElementExitFn(stylingApply); } - markStylingStateAsDirty(); return updated; } @@ -384,9 +378,9 @@ function updateDirectiveInputValue( const initialValue = getInitialStylingValue(context); const value = normalizeStylingDirectiveInputValue(initialValue, newValue, isClassBased); setInputsForProperty(lView, inputs, value); + setElementExitFn(stylingApply); } setValue(lView, bindingIndex, newValue); - setElementExitFn(applyStyling); } } @@ -423,17 +417,17 @@ function normalizeStylingDirectiveInputValue( * in this file. When called it will flush all style and class bindings to the element * via the context resolution algorithm. */ -function applyStyling(): void { - const elementIndex = getSelectedIndex(); +function stylingApply(): void { const lView = getLView(); + const elementIndex = getSelectedIndex(); const tNode = getTNode(elementIndex, lView); - const renderer = getRenderer(tNode, lView); const native = getNativeByTNode(tNode, lView) as RElement; + const directiveIndex = getActiveDirectiveId(); + const renderer = getRenderer(tNode, lView); const sanitizer = getCurrentStyleSanitizer(); const classesContext = isStylingContext(tNode.classes) ? tNode.classes as TStylingContext : null; const stylesContext = isStylingContext(tNode.styles) ? tNode.styles as TStylingContext : null; - flushStyling( - renderer, lView, classesContext, stylesContext, native, getActiveDirectiveId(), sanitizer); + flushStyling(renderer, lView, classesContext, stylesContext, native, directiveIndex, sanitizer); setCurrentStyleSanitizer(null); } @@ -538,10 +532,6 @@ function resolveStylePropValue( return resolvedValue; } -function markStylingStateAsDirty(): void { - setActiveElementFlag(ActiveElementFlags.ResetStylesOnExit); -} - /** * Whether or not the style/class binding being applied was executed within a host bindings * function. diff --git a/packages/core/test/acceptance/styling_next_spec.ts b/packages/core/test/acceptance/styling_next_spec.ts index 392b996974..fbc5f01d1c 100644 --- a/packages/core/test/acceptance/styling_next_spec.ts +++ b/packages/core/test/acceptance/styling_next_spec.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, HostBinding, Input, ViewChild} from '@angular/core'; +import {Component, ComponentFactoryResolver, ComponentRef, Directive, HostBinding, Input, NgModule, ViewChild, ViewContainerRef} from '@angular/core'; import {DebugNode, LViewDebug, toDebug} from '@angular/core/src/render3/instructions/lview_debug'; import {loadLContextFromNode} from '@angular/core/src/render3/util/discovery_utils'; import {ngDevModeResetPerfCounters as resetStylingCounters} from '@angular/core/src/util/ng_dev_mode'; @@ -1168,6 +1168,175 @@ describe('new styling integration', () => { expect(div.classList.contains('foo')).toBeTruthy(); expect(div.classList.contains('bar')).toBeTruthy(); }); + + it('should allow detectChanges to be run in a property change that causes additional styling to be rendered', + () => { + @Component({ + selector: 'child', + template: ` +
+ `, + }) + class ChildCmp { + readyTpl = false; + + @HostBinding('class.ready-host') + readyHost = false; + } + + @Component({ + selector: 'parent', + template: ` +
+
+

{{prop}}

+
+ `, + host: { + '[style.color]': 'color', + }, + }) + class ParentCmp { + private _prop = ''; + + @ViewChild('template', {read: ViewContainerRef, static: false}) + vcr: ViewContainerRef = null !; + + private child: ComponentRef = null !; + + @Input() + set prop(value: string) { + this._prop = value; + if (this.child && value === 'go') { + this.child.instance.readyHost = true; + this.child.instance.readyTpl = true; + this.child.changeDetectorRef.detectChanges(); + } + } + + get prop() { return this._prop; } + + ngAfterViewInit() { + const factory = this.componentFactoryResolver.resolveComponentFactory(ChildCmp); + this.child = this.vcr.createComponent(factory); + } + + constructor(private componentFactoryResolver: ComponentFactoryResolver) {} + } + + @Component({ + template: ``, + }) + class App { + prop = 'a'; + } + + @NgModule({ + entryComponents: [ChildCmp], + declarations: [ChildCmp], + }) + class ChildCmpModule { + } + + TestBed.configureTestingModule({declarations: [App, ParentCmp], imports: [ChildCmpModule]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(false); + + let readyHost = fixture.nativeElement.querySelector('.ready-host'); + let readyChild = fixture.nativeElement.querySelector('.ready-child'); + + expect(readyHost).toBeFalsy(); + expect(readyChild).toBeFalsy(); + + fixture.componentInstance.prop = 'go'; + fixture.detectChanges(false); + + readyHost = fixture.nativeElement.querySelector('.ready-host'); + readyChild = fixture.nativeElement.querySelector('.ready-child'); + expect(readyHost).toBeTruthy(); + expect(readyChild).toBeTruthy(); + }); + + it('should allow detectChanges to be run in a hook that causes additional styling to be rendered', + () => { + @Component({ + selector: 'child', + template: ` +
+ `, + }) + class ChildCmp { + readyTpl = false; + + @HostBinding('class.ready-host') + readyHost = false; + } + + @Component({ + selector: 'parent', + template: ` +
+
+

{{prop}}

+
+ `, + }) + class ParentCmp { + updateChild = false; + + @ViewChild('template', {read: ViewContainerRef, static: false}) + vcr: ViewContainerRef = null !; + + private child: ComponentRef = null !; + + ngDoCheck() { + if (this.updateChild) { + this.child.instance.readyHost = true; + this.child.instance.readyTpl = true; + this.child.changeDetectorRef.detectChanges(); + } + } + + ngAfterViewInit() { + const factory = this.componentFactoryResolver.resolveComponentFactory(ChildCmp); + this.child = this.vcr.createComponent(factory); + } + + constructor(private componentFactoryResolver: ComponentFactoryResolver) {} + } + + @Component({ + template: ``, + }) + class App { + @ViewChild('parent', {static: true}) public parent: ParentCmp|null = null; + } + + @NgModule({ + entryComponents: [ChildCmp], + declarations: [ChildCmp], + }) + class ChildCmpModule { + } + + TestBed.configureTestingModule({declarations: [App, ParentCmp], imports: [ChildCmpModule]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(false); + + let readyHost = fixture.nativeElement.querySelector('.ready-host'); + let readyChild = fixture.nativeElement.querySelector('.ready-child'); + expect(readyHost).toBeFalsy(); + expect(readyChild).toBeFalsy(); + + const parent = fixture.componentInstance.parent !; + parent.updateChild = true; + fixture.detectChanges(false); + + readyHost = fixture.nativeElement.querySelector('.ready-host'); + readyChild = fixture.nativeElement.querySelector('.ready-child'); + expect(readyHost).toBeTruthy(); + expect(readyChild).toBeTruthy(); + }); }); function assertStyleCounters(countForSet: number, countForRemove: number) { diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index f3a00560ea..11fcec25b5 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -167,9 +167,6 @@ { "name": "_selectedIndex" }, - { - "name": "_state" - }, { "name": "addComponentLogic" }, @@ -590,9 +587,6 @@ { "name": "resetPreOrderHookFlags" }, - { - "name": "resetStylingState" - }, { "name": "resolveDirectives" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 441806495d..11b96b90b7 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -146,9 +146,6 @@ { "name": "_selectedIndex" }, - { - "name": "_state" - }, { "name": "addToViewTree" }, @@ -434,9 +431,6 @@ { "name": "resetPreOrderHookFlags" }, - { - "name": "resetStylingState" - }, { "name": "selectIndexInternal" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index e617d0c535..3654a26b81 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -467,9 +467,6 @@ { "name": "applyProjectionRecursive" }, - { - "name": "applyStyling" - }, { "name": "applyStylingValue" }, @@ -914,9 +911,6 @@ { "name": "hasConfig" }, - { - "name": "hasDirectives" - }, { "name": "hasParentInjector" }, @@ -945,7 +939,7 @@ "name": "initNodeFlags" }, { - "name": "initializeTNodeInputs" + "name": "initializeInputAndOutputAliases" }, { "name": "injectElementRef" @@ -1106,9 +1100,6 @@ { "name": "markDirtyIfOnPush" }, - { - "name": "markStylingStateAsDirty" - }, { "name": "markViewDirty" }, @@ -1355,6 +1346,9 @@ { "name": "stringifyForError" }, + { + "name": "stylingApply" + }, { "name": "stylingMapToString" },