From c3967bd964c3517c60200a3241a7647526f47069 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Fri, 17 Mar 2017 13:18:43 -0500 Subject: [PATCH] fix(ui-sref): Improve performance of generating hrefs Closes https://github.com/angular-ui/ui-router/issues/3361 --- src/params/param.ts | 59 +++++++++++++++++++++--------- src/state/stateService.ts | 75 +++++++++++++++++++-------------------- src/url/urlMatcher.ts | 23 +++++++----- src/url/urlRouter.ts | 4 +-- 4 files changed, 96 insertions(+), 65 deletions(-) diff --git a/src/params/param.ts b/src/params/param.ts index cbe752de..e1d0ec55 100644 --- a/src/params/param.ts +++ b/src/params/param.ts @@ -2,9 +2,9 @@ * @coreapi * @module params */ /** for typedoc */ -import { extend, filter, map, applyPairs, allTrueR } from "../common/common"; -import { prop, propEq } from "../common/hof"; -import { isInjectable, isDefined, isString, isArray } from "../common/predicates"; +import { extend, filter, map, allTrueR } from "../common/common"; +import { prop } from "../common/hof"; +import { isInjectable, isDefined, isString, isArray, isUndefined } from "../common/predicates"; import { RawParams, ParamDeclaration } from "../params/interface"; import { services } from "../common/coreservices"; import { ParamType } from "./paramType"; @@ -17,15 +17,22 @@ import { UrlMatcherFactory } from "../url/urlMatcherFactory"; /** @internalapi */ export enum DefType { - PATH, SEARCH, CONFIG + PATH, + SEARCH, + CONFIG, } /** @hidden */ function unwrapShorthand(cfg: ParamDeclaration): ParamDeclaration { cfg = isShorthand(cfg) && { value: cfg } as any || cfg; + getStaticDefaultValue['__cacheable'] = true; + function getStaticDefaultValue() { + return cfg.value; + } + return extend(cfg, { - $$fn: isInjectable(cfg.value) ? cfg.value : () => cfg.value + $$fn: isInjectable(cfg.value) ? cfg.value : getStaticDefaultValue, }); } @@ -59,7 +66,7 @@ function getSquashPolicy(config: ParamDeclaration, isOptional: boolean, defaultP function getReplace(config: ParamDeclaration, arrayMode: boolean, isOptional: boolean, squash: (string|boolean)) { let replace: any, configuredKeys: string[], defaultPolicy = [ {from: "", to: (isOptional || arrayMode ? undefined : "")}, - {from: null, to: (isOptional || arrayMode ? undefined : "")} + {from: null, to: (isOptional || arrayMode ? undefined : "")}, ]; replace = isArray(config.replace) ? config.replace : []; if (isString(squash)) replace.push({ from: squash, to: undefined }); @@ -77,10 +84,14 @@ export class Param { dynamic: boolean; raw: boolean; squash: (boolean|string); - replace: any; + replace: [{ to: any, from: any }]; inherit: boolean; array: boolean; config: any; + /** Cache the default value if it is a static value */ + _defaultValueCache: { + defaultValue: any, + }; constructor(id: string, type: ParamType, config: ParamDeclaration, location: DefType, urlMatcherFactory: UrlMatcherFactory) { config = unwrapShorthand(config); @@ -101,7 +112,7 @@ export class Param { return extend(arrayDefaults, arrayParamNomenclature, config).array; } - extend(this, {id, type, location, isOptional, dynamic, raw, squash, replace, inherit, array: arrayMode, config, }); + extend(this, {id, type, location, isOptional, dynamic, raw, squash, replace, inherit, array: arrayMode, config }); } isDefaultValue(value: any): boolean { @@ -116,21 +127,33 @@ export class Param { /** * [Internal] Get the default value of a parameter, which may be an injectable function. */ - const $$getDefaultValue = () => { + const getDefaultValue = () => { + if (this._defaultValueCache) return this._defaultValueCache.defaultValue; + if (!services.$injector) throw new Error("Injectable functions cannot be called at configuration time"); + let defaultValue = services.$injector.invoke(this.config.$$fn); + if (defaultValue !== null && defaultValue !== undefined && !this.type.is(defaultValue)) throw new Error(`Default value (${defaultValue}) for parameter '${this.id}' is not an instance of ParamType (${this.type.name})`); + + if (this.config.$$fn['__cacheable']) { + this._defaultValueCache = { defaultValue }; + } + return defaultValue; }; - const $replace = (val: any) => { - let replacement: any = map(filter(this.replace, propEq('from', val)), prop("to")); - return replacement.length ? replacement[0] : val; + const replaceSpecialValues = (val: any) => { + for (let tuple of this.replace) { + if (tuple.from === val) return tuple.to; + } + return val; }; - value = $replace(value); - return !isDefined(value) ? $$getDefaultValue() : this.type.$normalize(value); + value = replaceSpecialValues(value); + + return isUndefined(value) ? getDefaultValue() : this.type.$normalize(value); } isSearch(): boolean { @@ -139,7 +162,7 @@ export class Param { validates(value: any): boolean { // There was no parameter value, but the param is optional - if ((!isDefined(value) || value === null) && this.isOptional) return true; + if ((isUndefined(value) || value === null) && this.isOptional) return true; // The value was not of the correct ParamType, and could not be decoded to the correct ParamType const normalized = this.type.$normalize(value); @@ -155,7 +178,11 @@ export class Param { } static values(params: Param[], values: RawParams = {}): RawParams { - return params.map(param => [param.id, param.value(values[param.id])]).reduce(applyPairs, {}); + const paramValues = {} as RawParams; + for (let param of params) { + paramValues[param.id] = param.value(values[param.id]); + } + return paramValues; } /** diff --git a/src/state/stateService.ts b/src/state/stateService.ts index f18dc77a..31a0d86c 100644 --- a/src/state/stateService.ts +++ b/src/state/stateService.ts @@ -1,37 +1,34 @@ /** * @coreapi * @module state - */ /** */ -import { - extend, defaults, silentRejection, silenceUncaughtInPromise, removeFrom, noop, createProxyFunctions, inArray -} from "../common/common"; -import {isDefined, isObject, isString} from "../common/predicates"; -import {Queue} from "../common/queue"; -import {services} from "../common/coreservices"; - -import {PathFactory} from "../path/pathFactory"; -import {PathNode} from "../path/node"; - -import {TransitionOptions, HookResult} from "../transition/interface"; -import {defaultTransOpts} from "../transition/transitionService"; -import {Rejection, RejectType} from "../transition/rejectFactory"; -import {Transition} from "../transition/transition"; - -import {StateOrName, StateDeclaration, TransitionPromise, LazyLoadResult} from "./interface"; -import {StateObject} from "./stateObject"; -import {TargetState} from "./targetState"; - -import {RawParams} from "../params/interface"; -import {ParamsOrArray} from "../params/interface"; -import {Param} from "../params/param"; -import {Glob} from "../common/glob"; -import {HrefOptions} from "./interface"; -import {UIRouter} from "../router"; -import {UIInjector} from "../interface"; -import {ResolveContext} from "../resolve/resolveContext"; -import {StateParams} from "../params/stateParams"; // has or is using -import {lazyLoadState} from "../hooks/lazyLoad"; -import { val, not } from "../common/hof"; + */ +/** */ +import { createProxyFunctions, defaults, extend, inArray, noop, removeFrom, silenceUncaughtInPromise, silentRejection } from '../common/common'; +import { isDefined, isObject, isString } from '../common/predicates'; +import { Queue } from '../common/queue'; +import { services } from '../common/coreservices'; + +import { PathFactory } from '../path/pathFactory'; +import { PathNode } from '../path/node'; + +import { HookResult, TransitionOptions } from '../transition/interface'; +import { defaultTransOpts } from '../transition/transitionService'; +import { Rejection, RejectType } from '../transition/rejectFactory'; +import { Transition } from '../transition/transition'; + +import { HrefOptions, LazyLoadResult, StateDeclaration, StateOrName, TransitionPromise } from './interface'; +import { StateObject } from './stateObject'; +import { TargetState } from './targetState'; + +import { ParamsOrArray, RawParams } from '../params/interface'; +import { Param } from '../params/param'; +import { Glob } from '../common/glob'; +import { UIRouter } from '../router'; +import { UIInjector } from '../interface'; +import { ResolveContext } from '../resolve/resolveContext'; +import { lazyLoadState } from '../hooks/lazyLoad'; +import { not, val } from '../common/hof'; +import { StateParams } from '../params/stateParams'; export type OnInvalidCallback = (toState?: TargetState, fromState?: TargetState, injector?: UIInjector) => HookResult; @@ -51,25 +48,25 @@ export class StateService { * * This is a passthrough through to [[UIRouterGlobals.transition]] */ - get transition() { return this.router.globals.transition; } + get transition() { return this.router.globals.transition; } /** * The latest successful state parameters * * This is a passthrough through to [[UIRouterGlobals.params]] */ - get params() { return this.router.globals.params; } + get params(): StateParams { return this.router.globals.params; } /** * The current [[StateDeclaration]] * * This is a passthrough through to [[UIRouterGlobals.current]] */ - get current() { return this.router.globals.current; } + get current() { return this.router.globals.current; } /** * The current [[StateObject]] * * This is a passthrough through to [[UIRouterGlobals.$current]] */ - get $current() { return this.router.globals.$current; } + get $current() { return this.router.globals.$current; } /** @internalapi */ constructor(private router: UIRouter) { @@ -208,7 +205,7 @@ export class StateService { return this.transitionTo(this.current, this.params, { reload: isDefined(reloadState) ? reloadState : true, inherit: false, - notify: false + notify: false, }); }; @@ -499,7 +496,7 @@ export class StateService { lossy: true, inherit: true, absolute: false, - relative: this.$current + relative: this.$current, }; options = defaults(options, defaultHrefOpts); params = params || {}; @@ -514,8 +511,8 @@ export class StateService { if (!nav || nav.url === undefined || nav.url === null) { return null; } - return this.router.urlRouter.href(nav.url, Param.values(state.parameters(), params), { - absolute: options.absolute + return this.router.urlRouter.href(nav.url, params, { + absolute: options.absolute, }); }; diff --git a/src/url/urlMatcher.ts b/src/url/urlMatcher.ts index a005f783..fa27dd23 100644 --- a/src/url/urlMatcher.ts +++ b/src/url/urlMatcher.ts @@ -333,7 +333,7 @@ export class UrlMatcher { */ parameters(opts: any = {}): Param[] { if (opts.inherit === false) return this._params; - return unnest(this._cache.path.map(prop('_params'))); + return unnest(this._cache.path.map(matcher => matcher._params)); } /** @@ -345,12 +345,14 @@ export class UrlMatcher { * @returns {T|Param|any|boolean|UrlMatcher|null} */ parameter(id: string, opts: any = {}): Param { + const findParam = () => { + for (let param of this._params) { + if (param.id === id) return param; + } + }; + let parent = this._cache.parent; - return ( - find(this._params, propEq('id', id)) || - (opts.inherit !== false && parent && parent.parameter(id, opts)) || - null - ); + return findParam() || (opts.inherit !== false && parent && parent.parameter(id, opts)) || null; } /** @@ -363,9 +365,14 @@ export class UrlMatcher { * @returns Returns `true` if `params` validates, otherwise `false`. */ validates(params: RawParams): boolean { - const validParamVal = (param: Param, val: any) => + const validParamVal = (param: Param, val: any) => !param || param.validates(val); - return pairs(params || {}).map(([key, val]) => validParamVal(this.parameter(key), val)).reduce(allTrueR, true); + + params = params || {}; + + // I'm not sure why this checks only the param keys passed in, and not all the params known to the matcher + let paramSchema = this.parameters().filter(paramDef => params.hasOwnProperty(paramDef.id)); + return paramSchema.map(paramDef => validParamVal(paramDef, params[paramDef.id])).reduce(allTrueR, true); } /** diff --git a/src/url/urlRouter.ts b/src/url/urlRouter.ts index 9206cae5..ac43a8b7 100644 --- a/src/url/urlRouter.ts +++ b/src/url/urlRouter.ts @@ -214,9 +214,9 @@ export class UrlRouter implements UrlRulesApi, UrlSyncApi, Disposable { * @returns Returns the fully compiled URL, or `null` if `params` fail validation against `urlMatcher` */ href(urlMatcher: UrlMatcher, params?: any, options?: { absolute: boolean }): string { - if (!urlMatcher.validates(params)) return null; - let url = urlMatcher.format(params); + if (url == null) return null; + options = options || { absolute: false }; let cfg = this._router.urlService.config;