Niedzielski has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/395810 )
Change subject: Chore: rename PageResolver
......................................................................
Chore: rename PageResolver
• Rename PageResolver from verb to noun, RequestPageModule.
• Use request / response instead of get or import to indicate that a
module is requested, the request may fail, and the implementation may
not be via import. Use page instead of name to match Route.page()
naming.
• Move RequestPageModule to the top of the file since the type is used
quite near the top.
• Add typing to requestPageModuleChunk since it otherwise returns a
Promise<any> and it's also top-level (although not exported).
Change-Id: I6983d1610fc7bf1744770e205aa74370e0288100
---
M docs/development.md
M src/common/router/router.test.ts
M src/common/router/router.ts
3 files changed, 20 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/marvin refs/changes/10/395810/1
diff --git a/docs/development.md b/docs/development.md
index 3a4863f..9bba644 100644
--- a/docs/development.md
+++ b/docs/development.md
@@ -176,6 +176,8 @@
- Static constants should be written in `SHOUTING_SNAKE_CASE`. All other
variables should be written in `camelCase`.
- Preact components should be written in PascalCase.
+- Functions should be verbs like `request` or `get`, not `requester` or
+ `getter`.
### CSS
diff --git a/src/common/router/router.test.ts b/src/common/router/router.test.ts
index 58a1f1e..b301a69 100644
--- a/src/common/router/router.test.ts
+++ b/src/common/router/router.test.ts
@@ -39,7 +39,7 @@
// eslint-disable-next-line max-len
it("throws redirect errors up for handling on the server/client
environment", done => {
// Page module that throws a redirect
- const page: PageModule<undefined, undefined> = {
+ const module: PageModule<undefined, undefined> = {
default: {
getInitialProps() {
// Trick TS and eslint for tests
@@ -50,9 +50,9 @@
}
};
- const getPage = (name: string) =>
- name === "redirect"
- ? Promise.resolve(page)
+ const requestPageModule = (page: string) =>
+ page === "redirect"
+ ? Promise.resolve(module)
: Promise.reject(new Error("No page found"));
const routes = [
@@ -62,7 +62,7 @@
})
];
- newRouter(routes, getPage)
+ newRouter(routes, requestPageModule)
.route("/redirect")
.catch(err => {
assert.ok(
diff --git a/src/common/router/router.ts b/src/common/router/router.ts
index c4b8f45..f208e5b 100644
--- a/src/common/router/router.ts
+++ b/src/common/router/router.ts
@@ -18,6 +18,10 @@
props: Props;
}
+interface RequestPageModule {
+ (name: string): Promise<PageModule<any, any>>;
+}
+
function getInitialProps<Params, Props>(
module: PageComponent<Params, Props>,
params: Params
@@ -29,18 +33,20 @@
/**
* Imports a page module from common/pages/* and names the chunk pages/* so
that
- * the router can tell the server the name of the chunks to preload
+ * the router can tell the server the name of the chunks to preload.
+ * @param {string} page The page chunk basename with no extension. Corresponds
+ * to Route.page.
*/
-function getChunk(name: string) {
- return import(/* webpackChunkName: "pages/[request]" */ `../pages/${name}`);
+function requestPageModuleChunk(page: string): Promise<PageModule<any, any>> {
+ return import(/* webpackChunkName: "pages/[request]" */ `../pages/${page}`);
}
function respond<Params, Props>(
- getPage: PageResolver,
+ requestPageModule: RequestPageModule,
route: Route<Params>,
params: Params
): Promise<RouteResponse<Props>> {
- return getPage(route.page).then(module =>
+ return requestPageModule(route.page).then(module =>
getInitialProps(
module.default,
params
@@ -67,20 +73,16 @@
};
}
-interface PageResolver {
- (name: string): Promise<PageModule<any, any>>;
-}
-
export const newRouter = (
routes: AnyRoute[],
- getPage: PageResolver = getChunk
+ requestPageModule: RequestPageModule = requestPageModuleChunk
) => {
return {
route(path: string): Promise<RouteResponse<any>> {
for (const route of routes) {
const params = route.toParams(path);
if (params) {
- return respond(getPage, route, params).catch(respondError);
+ return respond(requestPageModule, route, params).catch(respondError);
}
}
return Promise.resolve({
--
To view, visit https://gerrit.wikimedia.org/r/395810
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6983d1610fc7bf1744770e205aa74370e0288100
Gerrit-PatchSet: 1
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: Sniedzielski <sniedziel...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
