Niedzielski has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/381916 )
Change subject: Chore: consolidate path matching in Route
......................................................................
Chore: consolidate path matching in Route
- Refactor Router path matching logic into Route.toParams(). Route
already has access to the fundamental RouteConfig when it's built and
shouldn't really expose intermediates like the URL parameter names or
regular expression. The new design keeps tightly coupled code close
and simplifies the API tests.
- Rename Route.url() to toPath() since it returns a URL path, not a URL.
Change-Id: I111d9fdf852baf39da0c60d3afa56f9387281f7e
---
M src/common/components/header/header.tsx
M src/common/pages/home.tsx
M src/common/routers/api.test.ts
M src/common/routers/route.ts
M src/common/routers/router.ts
5 files changed, 57 insertions(+), 57 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/marvin refs/changes/16/381916/1
diff --git a/src/common/components/header/header.tsx
b/src/common/components/header/header.tsx
index c7eb263..afd2196 100644
--- a/src/common/components/header/header.tsx
+++ b/src/common/components/header/header.tsx
@@ -11,7 +11,7 @@
<div className="Header">
<div className="Header-left">
<Icon svg={menu} />
- <Link href={home.url()} class="Header-wordmark">
+ <Link href={home.toPath()} class="Header-wordmark">
<Wordmark />
</Link>
</div>
diff --git a/src/common/pages/home.tsx b/src/common/pages/home.tsx
index 457f8a4..6de4511 100644
--- a/src/common/pages/home.tsx
+++ b/src/common/pages/home.tsx
@@ -11,32 +11,32 @@
<p>Here are some test links for the time being:</p>
<ul>
<li>
- <Link href={home.url()}>Home</Link>
+ <Link href={home.toPath()}>Home</Link>
</li>
<li>
- <Link href={about.url()}>About</Link>
+ <Link href={about.toPath()}>About</Link>
</li>
<li>
- <Link href={styleGuide.url()}>Style Guide</Link>
+ <Link href={styleGuide.toPath()}>Style Guide</Link>
</li>
<li>
- <Link href={summary.url({ title: "Banana" })}>Banana</Link>
+ <Link href={summary.toPath({ title: "Banana" })}>Banana</Link>
</li>
<li>
<Link
- href={summary.url({ title: "Bill_&_Ted's_Excellent_Adventure" })}
+ href={summary.toPath({ title: "Bill_&_Ted's_Excellent_Adventure"
})}
>
Bill & Ted's Excellent Adventure
</Link>
</li>
<li>
- <Link href={summary.url({ title: "Cucumber" })}>Cucumber</Link>
+ <Link href={summary.toPath({ title: "Cucumber" })}>Cucumber</Link>
</li>
<li>
- <Link href={summary.url({ title: "Ice_cream" })}>Ice cream</Link>
+ <Link href={summary.toPath({ title: "Ice_cream" })}>Ice cream</Link>
</li>
<li>
- <Link href={summary.url({ title: "Plaintext" })}>
+ <Link href={summary.toPath({ title: "Plaintext" })}>
Article without image
</Link>
</li>
diff --git a/src/common/routers/api.test.ts b/src/common/routers/api.test.ts
index 72361a7..fed181b 100644
--- a/src/common/routers/api.test.ts
+++ b/src/common/routers/api.test.ts
@@ -12,26 +12,19 @@
params: Params;
}) =>
it(name, () => {
- const url = route.url(params);
- const [, ...paramValues] = route.pathRe.exec(url) || [];
- assert.ok(
- route.paramNames.length ===
- Object.keys((params as RouteParams) || {}).length
- );
- route.paramNames.forEach((key, index) => {
- const expected = encodeURIComponent((params as RouteParams)[key.name]);
- const actual = paramValues[index];
- assert.deepStrictEqual(
- actual,
- expected,
- `${key.name} value differs. Expected: ${expected}; actual: ${actual}.`
- );
+ const expected: RouteParams = {};
+ Object.keys((params as RouteParams) || {}).forEach(name => {
+ const value = (params as RouteParams)[name];
+ expected[name] = encodeURIComponent(value);
});
+
+ const path = route.toPath(params);
+ const result = route.toParams(path);
+ assert.deepStrictEqual(result, expected);
});
describe("api", () => {
- // eslint-disable-next-line max-len
- describe("each route's path, URL parameters, and route parameters match:",
() => {
+ describe("each route's path and URL path parameters match:", () => {
[
{ name: "home", route: home, params: undefined },
{ name: "about", route: about, params: undefined },
diff --git a/src/common/routers/route.ts b/src/common/routers/route.ts
index bcdfe54..443cf75 100644
--- a/src/common/routers/route.ts
+++ b/src/common/routers/route.ts
@@ -60,21 +60,47 @@
> extends RouteConfig<Params, Props> {
status: number;
- /** A regular expression for matching a URL path and capturing RouteParams.
*/
- pathRe: RegExp;
+ /**
+ * Generate a Params object from a given URL path or void if the path does
not
+ * match. An empty object is returned for matching NoPropsRoutes.
+ */
+ toParams(path: string): Params | void; // eslint-disable-line
no-use-before-define
- /** Parameter names. These are used to generate a Params object. */
- paramNames: pathToRegExp.Key[];
-
- /** Generates a URL from RouteParams. */
- url(params: Params): string;
+ /** Generates a URL path from Params. */
+ toPath(params: Params): string;
}
export interface NoPropsRoute extends Route<undefined, undefined> {
- url(params?: undefined): string;
+ toPath(params?: undefined): string;
}
export type AnyRoute = Route<any, any>;
+
+// This function is tightly coupled with Route.path and the parameters supplied
+// to PageModule.getInitialProps. Route.path must use names that match the
+// typing for the parameters of PageModule.getInitialProps(). This method only
+// associates the names of Route.path with the values found in the matched URL
+// path.
+const toParams = ({
+ pathRegExp,
+ paramNames,
+ path
+}: {
+ pathRegExp: RegExp;
+ paramNames: pathToRegExp.Key[];
+ path: string;
+}) => {
+ const matches = pathRegExp.exec(path);
+ if (matches) {
+ const [, ...paramValues] = matches;
+ const params: RouteParams = {};
+ paramNames.forEach((param, index) => {
+ params[param.name] = paramValues[index];
+ });
+ return params;
+ }
+ return undefined;
+};
export function newRoute<
Params extends RouteParams | undefined = undefined,
@@ -86,13 +112,13 @@
status = 200
}: RouteConfig<Params, Props>): Route<Params, Props> {
const paramNames: pathToRegExp.Key[] = [];
+ const pathRegExp = pathToRegExp(path, paramNames);
return {
path,
importModule,
chunkName,
status,
- pathRe: pathToRegExp(path, paramNames),
- paramNames,
- url: pathToRegExp.compile(path)
+ toParams: (path: string) => toParams({ pathRegExp, paramNames, path }),
+ toPath: pathToRegExp.compile(path)
} as Route<Params, Props>;
}
diff --git a/src/common/routers/router.ts b/src/common/routers/router.ts
index c02fa1b..cc66e84 100644
--- a/src/common/routers/router.ts
+++ b/src/common/routers/router.ts
@@ -1,4 +1,3 @@
-import * as pathToRegExp from "path-to-regexp";
import { AnyComponent } from "../components/preact-utils";
import {
AnyRoute,
@@ -17,22 +16,6 @@
export interface Router {
route(path: string): Promise<RouteResponse<any>>;
}
-
-// This method is tightly coupled with Route.path and the parameters supplied
to
-// PageModule.getInitialProps. Route.path must use names that match the typing
-// for the parameters of PageModule.getInitialProps(). This method only
-// associates the names of Route.path with the values found in the matched URL.
-const newRouteParams = (
- paramNames: pathToRegExp.Key[],
- paramValues: string[]
-): RouteParams =>
- paramNames.reduce(
- (params: RouteParams, paramName: pathToRegExp.Key, index: number) => {
- params[paramName.name] = paramValues[index];
- return params;
- },
- {}
- );
function getInitialProps<Params extends RouteParams | undefined, Props>(
module: PageModule<Params, Props>,
@@ -61,10 +44,8 @@
return {
route(path) {
for (const route of routes) {
- const matches = route.pathRe.exec(path);
- if (matches) {
- const [, ...paramValues] = matches;
- const params = newRouteParams(route.paramNames, paramValues);
+ const params = route.toParams(path);
+ if (params) {
return respond(route, params);
}
}
--
To view, visit https://gerrit.wikimedia.org/r/381916
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I111d9fdf852baf39da0c60d3afa56f9387281f7e
Gerrit-PatchSet: 1
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
