Skip to content

Commit

Permalink
fix(core): initialize global ngDevMode without toplevel side effects (#…
Browse files Browse the repository at this point in the history
…32079)

Fix #31595

PR Close #32079
  • Loading branch information
filipesilva authored and matsko committed Sep 12, 2019
1 parent a85eccd commit 5f095a5
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 36 deletions.
3 changes: 2 additions & 1 deletion integration/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/render3/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -240,6 +238,10 @@ export function ɵɵdefineComponent<T>(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;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/empty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 22 additions & 13 deletions packages/core/src/render3/instructions/lview_debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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.
/**
Expand All @@ -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;
}

Expand Down Expand Up @@ -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.
Expand All @@ -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;



Expand Down
20 changes: 11 additions & 9 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1187,7 +1188,7 @@ function findDirectiveMatches(
for (let i = 0; i < registry.length; i++) {
const def = registry[i] as ComponentDef<any>| DirectiveDef<any>;
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)) {
Expand All @@ -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);
}

Expand All @@ -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
Expand Down Expand Up @@ -1380,7 +1381,7 @@ function setInputsFromAttrs<T>(
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);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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.
Expand Down Expand Up @@ -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() : []);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/render3/jit/directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<any>, 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;

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/util/empty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
43 changes: 39 additions & 4 deletions packages/core/src/util/ng_dev_mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
6 changes: 4 additions & 2 deletions packages/platform-browser/src/dom/dom_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down Expand Up @@ -221,15 +223,15 @@ 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;
}

setValue(node: any, value: string): void { node.nodeValue = value; }

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));
Expand Down

0 comments on commit 5f095a5

Please sign in to comment.