Skip to content
Permalink
Browse files

perf(core): be more consistent about typeof checks (#28400)

When testing whether `value` is an object, use the ideal sequence of
strictly not equal to `null` followed by `typeof value === 'object'`
consistently. Specifically there's no point in using double equal with
`null` since `undefined` is ruled out by the `typeof` check.

Also avoid the unnecessary ToBoolean check on `value.ngOnDestroy` in
`hasOnDestroy()`, since the `typeof value.ngOnDestroy === 'function'`
will only let closures pass and all closures are truish (with the
notable exception of `document.all`, but that shouldn't be relevant
for the `ngOnDestroy` hook).

PR Close #28400
  • Loading branch information...
bmeurer authored and jasonaden committed Jan 28, 2019
1 parent 2bb518c commit 9af18c2fd07129171fbacd528ad109adfdb9f784
@@ -435,7 +435,7 @@ function deepForEach<T>(input: (T | any[])[], fn: (value: T) => void): void {
}

function isValueProvider(value: SingleProvider): value is ValueProvider {
return value && typeof value == 'object' && USE_VALUE in value;
return value !== null && typeof value == 'object' && USE_VALUE in value;
}

function isExistingProvider(value: SingleProvider): value is ExistingProvider {
@@ -456,7 +456,7 @@ function hasDeps(value: ClassProvider | ConstructorProvider | StaticClassProvide
}

function hasOnDestroy(value: any): value is OnDestroy {
return typeof value === 'object' && value != null && (value as OnDestroy).ngOnDestroy &&
return value !== null && typeof value === 'object' &&
typeof(value as OnDestroy).ngOnDestroy === 'function';
}

@@ -248,7 +248,7 @@ export class NodeInjectorFactory {
const FactoryPrototype = NodeInjectorFactory.prototype;
export function isFactory(obj: any): obj is NodeInjectorFactory {
// See: https://jsperf.com/instanceof-vs-getprototypeof
return obj != null && typeof obj == 'object' && Object.getPrototypeOf(obj) == FactoryPrototype;
return obj !== null && typeof obj == 'object' && Object.getPrototypeOf(obj) == FactoryPrototype;
}

// Note: This hack is necessary so we don't erroneously get a circular dependency
@@ -158,7 +158,7 @@ function _createProviderInstance(ngModule: NgModuleData, providerDef: NgModulePr
// avoided if possible. The sequence of checks here determines whether ngOnDestroy needs to be
// checked. It might not if the `injectable` isn't an object or if NodeFlags.OnDestroy is already
// set (ngOnDestroy was detected statically).
if (injectable !== UNDEFINED_VALUE && injectable != null && typeof injectable === 'object' &&
if (injectable !== UNDEFINED_VALUE && injectable !== null && typeof injectable === 'object' &&
!(providerDef.flags & NodeFlags.OnDestroy) && typeof injectable.ngOnDestroy === 'function') {
providerDef.flags |= NodeFlags.OnDestroy;
}

0 comments on commit 9af18c2

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.