Fixes for Typed Params

[christopherthielen]

No description provided.

[christopherthielen]

protoKeys was necessary because I used prototypal inheritance for ParamsSet, like we discussed. objectKeys doesn't work for prototypal inheritance.

[christopherthielen]

rebased, squashed, etc.

Summary of changes is as follows:

---

c7530b6

• Param class:
  • Represents a parameter of a state.
  • Encapsulates: id, type, optionality, and default value
  • Constructor: (id, [optional] type, config)
    • Creates a Param object from 'config', with an optional Type defined from a URL placeholder {id:Type}
    • If a Type was referenced in a URL definition, the type is used. Else we use the type from config. Else we use a default type.
  • getDefaultValueConfig: figures out the default value based on the config.
    • If .value or .type is found, then it's not shorthand.
    • If the value found is a fn, return it. Else wrap the value as a fn and return it
• ParamSet class
  • Encapsulates a set of params as an assoc-array { paramid: Param }
  • $$keys, $$values(paramValues), $$equals(paramValues1, paramValues2), $$validates(paramValues)
  • Can be prototypally inherited

---

dbdda92

• populate default params in .transitionTo.
  • Call ParamSet.values() to get default values, then merge toParams into it.
  • (note: this is replaced in a later commit with a call to ParamSet.values(toParams) which does the same thing, but normalizes toParams to the correct Type)

closes #1396

---

100267c

• Add state params validation in transitionTo. This enables transitions to Fail if the { param: value } args do not validate against the params/type definitions.
• We need to discuss this, since the current expected behavior does not allow this to happen.

Closes #1433

---

8dd978d

• delay all state registration until runtime.

closes #1435

---

2371ba9

• revert to string coersion of params by default.
• Add default Types for path params and search/query params. Query params support array notation.

---

2158a9d

• Switch to explicit placeholder Type in state url definition
  • previous format: {param:regexOrType}
  • new format: {param:regex:Type} (or {param:regex} or {param::Type})

[christopherthielen]

@nateabele I'm going to argue for a couple of additional Type system changes:

optional params

I don't think optional params should remove their segments from the url.

See the following state definition

.state({
 name: 'foo',
 url: '/:a/:b/:c/:d',
 params: {  a: '1',  b: '2',  c: '3',  d: '4' }
});

Due to squashing of slashes in $urlMatcherFactory like so:if (!values) return segments.join('').replace('//', '/'); and result.replace('//', '/'), we can no longer generate go both directions, between url and state+params

$state.go("foo", { a: 5, d: 10 });
$stateParams; // { a: "5", b: "2", c: "3", d: "10" }
$location.url() === '#/foo/5/10'
$location.url($location.url()); // essentially, user refreshes their browser
$stateParams; // { a: "5", b: "10", c: "3", d: "4" }

I couldn't come up with any reasonable rules for matching missing params, that would make sense in all scenarios.

UrlMatcher.format() applying defaults

I'm considering if .format() should optionally encode defaults into the URL. For example,

$state.go("foo", { a: 5, d: 10 });
$stateParams; // { a: "5", b: "2", c: "3", d: "10" }
$location.url() === '#/foo/5/2/3/10'

I think some users will expect #/foo/5/2/3/10 and some will expect #/foo/5///10

---

Injecting Type definition

I do not think we should support injectable Type definition objects

Here is my argument:

After deferring state definition until Runtime in my PR, I loaded up the UI-Router sample app. It's doing: $urlRouterProvider.when('/c?id', '/contacts/:id') at config time, which uses UrlMatchers, which use the Type registry, which isn't populated until runtime. Thus, the sample app failed to init due to a missing "searchType" in the $types registry.

I have considered a few approaches to addressing this:

• adding only the default types at config time to allow the sample app to run correctly
  • this feels odd because only the default types (used when no type has been explicitly called out) are registered at config time, and then they're not even the same types at runtime if a user has overridden them.
• registering Types at config time, but deferring the definition until runtime via callbacks
  • this has the problem that
• delaying .when until runtime
  • why not just delay everything until runtime 👎 it feels like this one thing (injectable Type defs) is pushing a bunch of dependencies to runtime. Anything else that wants to use UrlMatcher will have to wait too.

Everything feels either just plain wrong or like a one-use-case workaround. I think I would prefer to define and register Types at config time. I don't think injectable type definitions are going to be common, and if somebody needs their encode or decode injected for whatever reason, we can point them to the $rti gist.

[christopherthielen]

I made $urlRouterProvider.when defer registration until runtime as well and pushed.

However, I ran across another use case that becomes problematic. $urlMatcherFactoryProvider can be used at config time to generate UrlMatcher(s). In fact, I have an open PR in ui-router-extras which does exactly this.

I know we discussed it in depth today, but I'm still not feeling good about deferring config-type-stuff to runtime. All I really need at config time is type.pattern.

So, I think I have a workable proposal:

Change from:
$urlMatcherFactoryProvider.type(name, objOrFn)
to:
$urlMatcherFactoryProvider.type(name, obj, runtimeInjectableFn)

• Register the types as usual during config phase.
• angular.extend() the registered type with the results of the injected fn.
• Use the pattern from the obj (or the default pattern on Type.prototype)

I like this because it doesn't jump through hoops, is easy to develop against, and smells better to me overall.

[nateabele]

👍