Skip to content

Commit

Permalink
refactor(router): address TS strictNullChecks, update types, add checks
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Route & RouteMatch IDs MUST be strings now

- update config fields from PropertyKey => string
- add initial & default route checks in ctor
  • Loading branch information
@postspectacular
postspectacular committed Jun 7, 2019
1 parent 1540f37 commit c7ff9a4
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 29 deletions.
8 changes: 4 additions & 4 deletions packages/router/src/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface RouteParamValidator {
* authentication. Apart from `id` and `match` all other fields
* are optional.
*/
export interface Route extends IID<PropertyKey> {
export interface Route extends IID<string> {
/**
* Array of path components. If a value is prefixed with `?` this
* path component will be captured under that name. E.g.
Expand Down Expand Up @@ -93,7 +93,7 @@ export interface Route extends IID<PropertyKey> {
* `EVENT_ROUTE_CHANGE`. Contains the matched route ID and any route
* params.
*/
export interface RouteMatch extends IID<PropertyKey> {
export interface RouteMatch extends IID<string> {
title?: string;

/**
Expand All @@ -116,12 +116,12 @@ export interface RouterConfig {
* defined routes could be matched against user input, e.g. a home
* or error page.
*/
defaultRouteID: PropertyKey;
defaultRouteID: string;
/**
* Optional initial route to trigger when router starts. If given,
* this MUST be a route without params.
*/
initialRouteID?: PropertyKey;
initialRouteID?: string;
/**
* Optional route authentication function. See `RouteAuthenticator`
* for further details. If no authenticator is given, all matched
Expand Down
55 changes: 38 additions & 17 deletions packages/router/src/basic.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
assert,
Event,
INotify,
INotifyMixin,
Expand All @@ -18,8 +19,8 @@ import {

@INotifyMixin
export class BasicRouter implements INotify {
public config: RouterConfig;
public current: RouteMatch;
config: RouterConfig;
current: RouteMatch | undefined;

constructor(config: RouterConfig) {
config.authenticator =
Expand All @@ -32,20 +33,37 @@ export class BasicRouter implements INotify {
config.prefix = config.prefix === undefined ? "/" : config.prefix;
config.separator = config.separator || "/";
this.config = config;
assert(
this.routeForID(this.config.defaultRouteID) !== undefined,
`missing config for default route: '${this.config.defaultRouteID}'`
);
if (config.initialRouteID) {
const route = this.routeForID(config.initialRouteID);
assert(
route !== undefined,
`missing config for initial route: ${
this.config.initialRouteID
}`
);
assert(
!isParametricRoute(route!),
"initial route MUST not be parametric"
);
}
}

// mixin
public addListener(_: string, __: Listener, ___?: any) {
addListener(_: string, __: Listener, ___?: any) {
return false;
}
public removeListener(_: string, __: Listener, ___?: any) {
removeListener(_: string, __: Listener, ___?: any) {
return false;
}
public notify(_: Event) {}
notify(_: Event) {}

start() {
if (this.config.initialRouteID) {
const route = this.routeForID(this.config.initialRouteID);
const route = this.routeForID(this.config.initialRouteID)!;
this.current = { id: route.id, title: route.title, params: {} };
this.notify({ id: EVENT_ROUTE_CHANGED, value: this.current });
}
Expand All @@ -59,13 +77,13 @@ export class BasicRouter implements INotify {
*
* @param raw route path to match
*/
route(src: string): RouteMatch {
route(src: string): RouteMatch | undefined {
if (src.charAt(0) === "#") {
src = src.substr(1);
}
src = src.substr(this.config.prefix.length);
src = src.substr(this.config.prefix!.length);
const routes = this.config.routes,
curr = src.split(this.config.separator);
curr = src.split(this.config.separator!);
let match;
for (let i = 0, n = routes.length; i < n; i++) {
const route = routes[i],
Expand All @@ -79,7 +97,7 @@ export class BasicRouter implements INotify {
if (!this.handleRouteFailure()) {
return;
}
const route = this.routeForID(this.config.defaultRouteID);
const route = this.routeForID(this.config.defaultRouteID)!;
match = { id: route.id, title: route.title, params: {} };
}
if (!equiv(match, this.current)) {
Expand All @@ -97,7 +115,7 @@ export class BasicRouter implements INotify {
* @param params
* @param hash if true, prepends `#` to results
*/
format(id: PropertyKey, params?: any, hash?: boolean): string;
format(id: string, params?: any, hash?: boolean): string;
format(match: Partial<RouteMatch>, hash?: boolean): string;
format(...args: any[]) {
let [id, params, hash] = args;
Expand All @@ -120,9 +138,9 @@ export class BasicRouter implements INotify {
default:
illegalArity(args.length);
}
const route = this.routeForID(match.id);
const route = this.routeForID(match!.id!);
if (route) {
const params = match.params || {};
const params = match!.params || {};
return (
(hash ? "#" : "") +
this.config.prefix +
Expand All @@ -137,15 +155,15 @@ export class BasicRouter implements INotify {
.join(this.config.separator)
);
} else {
illegalArgs(`invalid route ID: ${match.id.toString()}`);
illegalArgs(`invalid route ID: ${match!.id!}`);
}
}

routeForID(id: PropertyKey) {
routeForID(id: string) {
return this.config.routes.find((route) => route.id === id);
}

protected matchRoute(curr: string[], route: Route): RouteMatch {
protected matchRoute(curr: string[], route: Route): RouteMatch | undefined {
const match = route.match,
n = match.length;
if (curr.length === n) {
Expand All @@ -165,7 +183,7 @@ export class BasicRouter implements INotify {
return;
}
return route.auth
? this.config.authenticator(route, curr, params)
? this.config.authenticator!(route, curr, params)
: { id: route.id, title: route.title, params };
}
}
Expand All @@ -192,3 +210,6 @@ export class BasicRouter implements INotify {
return true;
}
}

const isParametricRoute = (route: Route) =>
route.match.some((p) => p.charAt(0) === "?");
17 changes: 9 additions & 8 deletions packages/router/src/history.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ import { HTMLRouterConfig, RouteMatch, RouterConfig } from "./api";
import { BasicRouter } from "./basic";

export class HTMLRouter extends BasicRouter {
protected currentPath: string;
protected popHandler: Fn<PopStateEvent, void>;
protected hashHandler: Fn<HashChangeEvent, void>;
protected currentPath!: string;
protected popHandler!: Fn<PopStateEvent, void>;
protected hashHandler!: Fn<HashChangeEvent, void>;
protected useFragment: boolean;
protected ignoreHashChange: boolean;

constructor(config: HTMLRouterConfig) {
super(<RouterConfig>config);
this.useFragment = config.useFragment !== false;
this.ignoreHashChange = false;
}

start() {
Expand All @@ -23,7 +24,7 @@ export class HTMLRouter extends BasicRouter {
window.addEventListener("hashchange", this.handleHashChange());
}
if (this.config.initialRouteID) {
const route = this.routeForID(this.config.initialRouteID);
const route = this.routeForID(this.config.initialRouteID)!;
this.route(
this.format({
id: route.id,
Expand Down Expand Up @@ -52,9 +53,9 @@ export class HTMLRouter extends BasicRouter {
* @param pushState
*/
route(src: string, pushState = true) {
const old = this.current,
route = super.route(src);
if (!equiv(route, old)) {
const old = this.current;
const route = super.route(src);
if (route && !equiv(route, old)) {
this.currentPath = this.format(route);
if (pushState) {
history.pushState(
Expand Down Expand Up @@ -119,7 +120,7 @@ export class HTMLRouter extends BasicRouter {
protected handleRouteFailure() {
this.ignoreHashChange = true;
location.hash = this.format({
id: this.routeForID(this.config.defaultRouteID).id
id: this.routeForID(this.config.defaultRouteID)!.id
});
this.ignoreHashChange = false;
return true;
Expand Down

0 comments on commit c7ff9a4

Please sign in to comment.