From 5f095a501eb23c27aea1425cbaa6aba7a3f9a664 Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Thu, 15 Aug 2019 14:14:25 +0100 Subject: [PATCH] fix(core): initialize global ngDevMode without toplevel side effects (#32079) Fix #31595 PR Close #32079 --- integration/_payload-limits.json | 3 +- packages/core/src/render3/definition.ts | 8 ++-- packages/core/src/render3/empty.ts | 4 +- .../src/render3/instructions/lview_debug.ts | 35 +++++++++------ .../core/src/render3/instructions/shared.ts | 20 +++++---- packages/core/src/render3/jit/directive.ts | 5 +++ packages/core/src/util/empty.ts | 4 +- packages/core/src/util/ng_dev_mode.ts | 43 +++++++++++++++++-- .../platform-browser/src/dom/dom_renderer.ts | 6 ++- 9 files changed, 92 insertions(+), 36 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 5d0ab0db8c..cf3b8943c8 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -39,7 +39,8 @@ "master": { "uncompressed": { "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", - "bundle": 177137 + "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", + "bundle": 177447 } } } diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 99672e8507..2b4b1aeff7 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -6,15 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import '../util/ng_dev_mode'; - import {ChangeDetectionStrategy} from '../change_detection/constants'; -import {NG_INJECTABLE_DEF, ɵɵdefineInjectable} from '../di/interface/defs'; import {Mutable, Type} from '../interface/type'; import {NgModuleDef} from '../metadata/ng_module'; import {SchemaMetadata} from '../metadata/schema'; import {ViewEncapsulation} from '../metadata/view'; import {noSideEffects} from '../util/closure'; +import {initNgDevMode} from '../util/ng_dev_mode'; import {stringify} from '../util/stringify'; import {EMPTY_ARRAY, EMPTY_OBJ} from './empty'; @@ -240,6 +238,10 @@ export function ɵɵdefineComponent(componentDefinition: { */ schemas?: SchemaMetadata[] | null; }): never { + // Initialize ngDevMode. This must be the first statement in ɵɵdefineComponent. + // See the `initNgDevMode` docstring for more information. + (typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode(); + const type = componentDefinition.type; const typePrototype = type.prototype; const declaredInputs: {[key: string]: string} = {} as any; diff --git a/packages/core/src/render3/empty.ts b/packages/core/src/render3/empty.ts index d0494193fd..36be572fbe 100644 --- a/packages/core/src/render3/empty.ts +++ b/packages/core/src/render3/empty.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 '../util/ng_dev_mode'; +import {initNgDevMode} from '../util/ng_dev_mode'; /** * This file contains reuseable "empty" symbols that can be used as default return values @@ -18,7 +18,7 @@ export const EMPTY_OBJ: {} = {}; export const EMPTY_ARRAY: any[] = []; // freezing the values prevents any code from accidentally inserting new values in -if (typeof ngDevMode !== 'undefined' && ngDevMode) { +if ((typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode()) { // These property accesses can be ignored because ngDevMode will be set to false // when optimizing code and the whole if statement will be dropped. // tslint:disable-next-line:no-toplevel-property-access diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index 1f5e3140e3..3e054571ae 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -10,6 +10,7 @@ import {AttributeMarker, ComponentTemplate} from '..'; import {SchemaMetadata} from '../../core'; import {assertDefined} from '../../util/assert'; import {createNamedArrayType} from '../../util/named_array_type'; +import {initNgDevMode} from '../../util/ng_dev_mode'; import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE} from '../interfaces/container'; import {DirectiveDefList, PipeDefList, ViewQueriesFunction} from '../interfaces/definition'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, TIcu} from '../interfaces/i18n'; @@ -24,6 +25,7 @@ import {isStylingContext} from '../styling_next/util'; import {attachDebugObject} from '../util/debug_utils'; import {getTNode, unwrapRNode} from '../util/view_utils'; +const NG_DEV_MODE = ((typeof ngDevMode === 'undefined' || !!ngDevMode) && initNgDevMode()); /* * This file contains conditionally attached classes which provide human readable (debug) level @@ -54,8 +56,7 @@ import {getTNode, unwrapRNode} from '../util/view_utils'; * ``` */ - -export const LViewArray = ngDevMode && createNamedArrayType('LView'); +export const LViewArray = NG_DEV_MODE && createNamedArrayType('LView') || null !as ArrayConstructor; let LVIEW_EMPTY: unknown[]; // can't initialize here or it will not be tree shaken, because `LView` // constructor could have side-effects. /** @@ -64,7 +65,7 @@ let LVIEW_EMPTY: unknown[]; // can't initialize here or it will not be tree sha * Simple slice will keep the same type, and we need it to be LView */ export function cloneToLView(list: any[]): LView { - if (LVIEW_EMPTY === undefined) LVIEW_EMPTY = new LViewArray !(); + if (LVIEW_EMPTY === undefined) LVIEW_EMPTY = new LViewArray(); return LVIEW_EMPTY.concat(list) as any; } @@ -195,7 +196,7 @@ function processTNodeChildren(tNode: TNode | null, buf: string[]) { } } -const TViewData = ngDevMode && createNamedArrayType('TViewData'); +const TViewData = NG_DEV_MODE && createNamedArrayType('TViewData') || null !as ArrayConstructor; let TVIEWDATA_EMPTY: unknown[]; // can't initialize here or it will not be tree shaken, because `LView` // constructor could have side-effects. @@ -205,18 +206,26 @@ let TVIEWDATA_EMPTY: * Simple slice will keep the same type, and we need it to be TData */ export function cloneToTViewData(list: any[]): TData { - if (TVIEWDATA_EMPTY === undefined) TVIEWDATA_EMPTY = new TViewData !(); + if (TVIEWDATA_EMPTY === undefined) TVIEWDATA_EMPTY = new TViewData(); return TVIEWDATA_EMPTY.concat(list) as any; } -export const LViewBlueprint = ngDevMode && createNamedArrayType('LViewBlueprint'); -export const MatchesArray = ngDevMode && createNamedArrayType('MatchesArray'); -export const TViewComponents = ngDevMode && createNamedArrayType('TViewComponents'); -export const TNodeLocalNames = ngDevMode && createNamedArrayType('TNodeLocalNames'); -export const TNodeInitialInputs = ngDevMode && createNamedArrayType('TNodeInitialInputs'); -export const TNodeInitialData = ngDevMode && createNamedArrayType('TNodeInitialData'); -export const LCleanup = ngDevMode && createNamedArrayType('LCleanup'); -export const TCleanup = ngDevMode && createNamedArrayType('TCleanup'); +export const LViewBlueprint = + NG_DEV_MODE && createNamedArrayType('LViewBlueprint') || null !as ArrayConstructor; +export const MatchesArray = + NG_DEV_MODE && createNamedArrayType('MatchesArray') || null !as ArrayConstructor; +export const TViewComponents = + NG_DEV_MODE && createNamedArrayType('TViewComponents') || null !as ArrayConstructor; +export const TNodeLocalNames = + NG_DEV_MODE && createNamedArrayType('TNodeLocalNames') || null !as ArrayConstructor; +export const TNodeInitialInputs = + NG_DEV_MODE && createNamedArrayType('TNodeInitialInputs') || null !as ArrayConstructor; +export const TNodeInitialData = + NG_DEV_MODE && createNamedArrayType('TNodeInitialData') || null !as ArrayConstructor; +export const LCleanup = + NG_DEV_MODE && createNamedArrayType('LCleanup') || null !as ArrayConstructor; +export const TCleanup = + NG_DEV_MODE && createNamedArrayType('TCleanup') || null !as ArrayConstructor; diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 05da849d7b..7d90d58944 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -12,6 +12,7 @@ import {validateAgainstEventAttributes, validateAgainstEventProperties} from '.. import {Sanitizer} from '../../sanitization/sanitizer'; import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame} from '../../util/assert'; import {createNamedArrayType} from '../../util/named_array_type'; +import {initNgDevMode} from '../../util/ng_dev_mode'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; import {assertFirstTemplatePass, assertLView} from '../assert'; import {attachPatchData, getComponentViewByInstance} from '../context_discovery'; @@ -656,7 +657,7 @@ export function createTView( } function createViewBlueprint(bindingStartIndex: number, initialViewLength: number): LView { - const blueprint = ngDevMode ? new LViewBlueprint !() : []; + const blueprint = ngDevMode ? new LViewBlueprint() : []; for (let i = 0; i < initialViewLength; i++) { blueprint.push(i < bindingStartIndex ? null : NO_CHANGE); @@ -1187,7 +1188,7 @@ function findDirectiveMatches( for (let i = 0; i < registry.length; i++) { const def = registry[i] as ComponentDef| DirectiveDef; if (isNodeMatchingSelectorList(tNode, def.selectors !, /* isProjectionMode */ false)) { - matches || (matches = ngDevMode ? new MatchesArray !() : []); + matches || (matches = ngDevMode ? new MatchesArray() : []); diPublicInInjector(getOrCreateNodeInjectorForNode(tNode, viewData), tView, def.type); if (isComponentDef(def)) { @@ -1212,7 +1213,7 @@ function findDirectiveMatches( export function markAsComponentHost(tView: TView, hostTNode: TNode): void { ngDevMode && assertFirstTemplatePass(tView); hostTNode.flags = TNodeFlags.isComponentHost; - (tView.components || (tView.components = ngDevMode ? new TViewComponents !() : [ + (tView.components || (tView.components = ngDevMode ? new TViewComponents() : [ ])).push(hostTNode.index); } @@ -1222,7 +1223,7 @@ function cacheMatchingLocalNames( tNode: TNode, localRefs: string[] | null, exportsMap: {[key: string]: number}): void { if (localRefs) { const localNames: (string | number)[] = tNode.localNames = - ngDevMode ? new TNodeLocalNames !() : []; + ngDevMode ? new TNodeLocalNames() : []; // Local names must be stored in tNode in the same order that localRefs are defined // in the template to ensure the data is loaded in the same slots as their refs @@ -1380,7 +1381,7 @@ function setInputsFromAttrs( function generateInitialInputs( directiveIndex: number, inputs: {[key: string]: string}, tNode: TNode): InitialInputData { const initialInputData: InitialInputData = - tNode.initialInputs || (tNode.initialInputs = ngDevMode ? new TNodeInitialInputs !() : []); + tNode.initialInputs || (tNode.initialInputs = ngDevMode ? new TNodeInitialInputs() : []); // Ensure that we don't create sparse arrays for (let i = initialInputData.length; i <= directiveIndex; i++) { initialInputData.push(null); @@ -1408,7 +1409,7 @@ function generateInitialInputs( if (minifiedInputName !== undefined) { const inputsToStore: InitialInputs = initialInputData[directiveIndex] || - (initialInputData[directiveIndex] = ngDevMode ? new TNodeInitialData !() : []); + (initialInputData[directiveIndex] = ngDevMode ? new TNodeInitialData() : []); inputsToStore.push(attrName as string, minifiedInputName, attrValue as string); } @@ -1422,7 +1423,8 @@ function generateInitialInputs( ////////////////////////// // Not sure why I need to do `any` here but TS complains later. -const LContainerArray: any = ngDevMode && createNamedArrayType('LContainer'); +const LContainerArray: any = ((typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode()) && + createNamedArrayType('LContainer'); /** * Creates a LContainer, either from a container instruction, or for a ViewContainerRef. @@ -1777,11 +1779,11 @@ export function initializeTNodeInputs(tView: TView, tNode: TNode): PropertyAlias export function getCleanup(view: LView): any[] { // top level variables should not be exported for performance reasons (PERF_NOTES.md) - return view[CLEANUP] || (view[CLEANUP] = ngDevMode ? new LCleanup !() : []); + return view[CLEANUP] || (view[CLEANUP] = ngDevMode ? new LCleanup() : []); } function getTViewCleanup(view: LView): any[] { - return view[TVIEW].cleanup || (view[TVIEW].cleanup = ngDevMode ? new TCleanup !() : []); + return view[TVIEW].cleanup || (view[TVIEW].cleanup = ngDevMode ? new TCleanup() : []); } /** diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index 334f937088..4d8b380c99 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -16,6 +16,7 @@ import {Query} from '../../metadata/di'; import {Component, Directive, Input} from '../../metadata/directives'; import {componentNeedsResolution, maybeQueueResolutionOfComponentResources} from '../../metadata/resource_loading'; import {ViewEncapsulation} from '../../metadata/view'; +import {initNgDevMode} from '../../util/ng_dev_mode'; import {getBaseDef, getComponentDef, getDirectiveDef} from '../definition'; import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty'; import {NG_BASE_DEF, NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_FACTORY_DEF} from '../fields'; @@ -37,6 +38,10 @@ import {flushModuleScopingQueueAsMuchAsPossible, patchComponentDefWithScope, tra * until the global queue has been resolved with a call to `resolveComponentResources`. */ export function compileComponent(type: Type, metadata: Component): void { + // Initialize ngDevMode. This must be the first statement in compileComponent. + // See the `initNgDevMode` docstring for more information. + (typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode(); + let ngComponentDef: any = null; let ngFactoryDef: any = null; diff --git a/packages/core/src/util/empty.ts b/packages/core/src/util/empty.ts index d1d8ce9a1b..d30b12e772 100644 --- a/packages/core/src/util/empty.ts +++ b/packages/core/src/util/empty.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 './ng_dev_mode'; +import {initNgDevMode} from './ng_dev_mode'; /** * This file contains reuseable "empty" symbols that can be used as default return values @@ -18,7 +18,7 @@ export const EMPTY_OBJ: {} = {}; export const EMPTY_ARRAY: any[] = []; // freezing the values prevents any code from accidentally inserting new values in -if (typeof ngDevMode !== 'undefined' && ngDevMode) { +if ((typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode()) { // These property accesses can be ignored because ngDevMode will be set to false // when optimizing code and the whole if statement will be dropped. // tslint:disable-next-line:no-toplevel-property-access diff --git a/packages/core/src/util/ng_dev_mode.ts b/packages/core/src/util/ng_dev_mode.ts index 505987072b..41c4e3fad0 100644 --- a/packages/core/src/util/ng_dev_mode.ts +++ b/packages/core/src/util/ng_dev_mode.ts @@ -9,6 +9,20 @@ import {global} from './global'; declare global { + /** + * Values of ngDevMode + * Depending on the current state of the application, ngDevMode may have one of several values. + * + * For convenience, the “truthy” value which enables dev mode is also an object which contains + * Angular’s performance counters. This is not necessary, but cuts down on boilerplate for the + * perf counters. + * + * ngDevMode may also be set to false. This can happen in one of a few ways: + * - The user explicitly sets `window.ngDevMode = false` somewhere in their app. + * - The user calls `enableProdMode()`. + * - The URL contains a `ngDevMode=false` text. + * Finally, ngDevMode may not have been defined at all. + */ const ngDevMode: null|NgDevModePerfCounters; interface NgDevModePerfCounters { namedConstructors: boolean; @@ -98,15 +112,36 @@ export function ngDevModeResetPerfCounters(): NgDevModePerfCounters { } /** - * This checks to see if the `ngDevMode` has been set. If yes, + * This function checks to see if the `ngDevMode` has been set. If yes, * then we honor it, otherwise we default to dev mode with additional checks. * * The idea is that unless we are doing production build where we explicitly * set `ngDevMode == false` we should be helping the developer by providing * as much early warning and errors as possible. * - * NOTE: changes to the `ngDevMode` name must be synced with `compiler-cli/src/tooling.ts`. + * `ɵɵdefineComponent` is guaranteed to have been called before any component template functions + * (and thus Ivy instructions), so a single initialization there is sufficient to ensure ngDevMode + * is defined for the entire instruction set. + * + * When using checking `ngDevMode` on toplevel, always init it before referencing it + * (e.g. `((typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode())`), otherwise you can + * get a `ReferenceError` like in https://github.com/angular/angular/issues/31595. + * + * Details on possible values for `ngDevMode` can be found on its docstring. + * + * NOTE: + * - changes to the `ngDevMode` name must be synced with `compiler-cli/src/tooling.ts`. */ -if (typeof ngDevMode === 'undefined' || ngDevMode) { - ngDevModeResetPerfCounters(); +export function initNgDevMode(): boolean { + // The below checks are to ensure that calling `initNgDevMode` multiple times does not + // reset the counters. + // If the `ngDevMode` is not an object, then it means we have not created the perf counters + // yet. + if (typeof ngDevMode === 'undefined' || ngDevMode) { + if (typeof ngDevMode !== 'object') { + ngDevModeResetPerfCounters(); + } + return !!ngDevMode; + } + return false; } diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts index 9f7b68ed2f..c25a18bfb3 100644 --- a/packages/platform-browser/src/dom/dom_renderer.ts +++ b/packages/platform-browser/src/dom/dom_renderer.ts @@ -20,6 +20,8 @@ export const NAMESPACE_URIS: {[ns: string]: string} = { }; const COMPONENT_REGEX = /%COMP%/g; +const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode; + export const COMPONENT_VARIABLE = '%COMP%'; export const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`; export const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`; @@ -221,7 +223,7 @@ class DefaultDomRenderer2 implements Renderer2 { } setProperty(el: any, name: string, value: any): void { - ngDevMode && checkNoSyntheticProp(name, 'property'); + NG_DEV_MODE && checkNoSyntheticProp(name, 'property'); el[name] = value; } @@ -229,7 +231,7 @@ class DefaultDomRenderer2 implements Renderer2 { listen(target: 'window'|'document'|'body'|any, event: string, callback: (event: any) => boolean): () => void { - ngDevMode && checkNoSyntheticProp(event, 'listener'); + NG_DEV_MODE && checkNoSyntheticProp(event, 'listener'); if (typeof target === 'string') { return <() => void>this.eventManager.addGlobalEventListener( target, event, decoratePreventDefault(callback));