jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/378267 )
Change subject: New: add page summaries fetcher and UI ...................................................................... New: add page summaries fetcher and UI - Add new UI for page summaries. - Fetch page summaries from RESTBase and display them. - It isn't clear why but the TypeScript root configuration file doesn't appear to pass includes to its children so copy this configuration downwards. (Copy instead of move so common code can still use the common config.) - DRY up "ChildrenProps" and "ClassProps" types for Preact components to allow children or a class parameter in a unified way. Also, break apart Children into ComponentChild and ComponentChildren. - Add isomorphic-unfetch to the package runtime dependencies, the vendor chunk, and add typing that mimics fetch. - Move Preact supplemental definitions to a plain TypeScript file so functional utilities can also be specified. - Minor hygiene to rename "parameters" to "params" and update the docs. - Add type aliases for page title URL path, decoded page URL path, and localized titles. These types all alias string but at least they can point to JSDocs and help devs consider the contents, avoid bugs, and clarify API expectations. - Add a couple promised Webpack configuration comments promised long ago. Bug: T173324 Change-Id: I7456e654e24db34343bd8e108bc990be8fff31fe --- M docs/development.md M package-lock.json M package.json M src/client/tsconfig.json M src/common/components/app/app.tsx M src/common/components/link.tsx A src/common/components/page-summary/page-summary.css A src/common/components/page-summary/page-summary.tsx M src/common/components/paper/paper.tsx A src/common/components/preact-utils.ts M src/common/components/with-context.ts A src/common/data-clients/page-data-client.ts M src/common/models/page.ts M src/common/pages/wiki.tsx M src/common/routers/api.ts M src/common/routers/route.ts M src/common/routers/router.ts A src/common/types/isomorphic-unfetch.d.ts D src/common/types/preact.d.ts M src/server/components/page.tsx M src/server/index.tsx M src/server/tsconfig.json M webpack.config.ts 23 files changed, 241 insertions(+), 56 deletions(-) Approvals: Jhernandez: Looks good to me, approved jenkins-bot: Verified diff --git a/docs/development.md b/docs/development.md index 41111d2..d7e34ef 100644 --- a/docs/development.md +++ b/docs/development.md @@ -124,8 +124,14 @@ - Static constants should be written in `SHOUTING_SNAKE_CASE`. All other variables should be written in `camelCase`. - Preact components should be written in PascalCase. -- Preact component properties variables and types, especially - `ComponentProps` implementations, should be called "props" and "Props". + +#### Abbreviations + +Marvin uses the following abbreviations: + +- Parameters => params +- Properties => props +- Utilities => utils ### Filenaming diff --git a/package-lock.json b/package-lock.json index 50bafa5..8136876 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1519,6 +1519,14 @@ "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-1.0.1.tgz", "integrity": "sha1-eePVhlU0aQn+bw9Fpd5oEDspTSA=" }, + "encoding": { + "version": "0.1.12", + "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.12.tgz", + "integrity": "sha1-U4tm8+5izRq1HsMjgp0flIDHS+s=", + "requires": { + "iconv-lite": "0.4.18" + } + }, "enhanced-resolve": { "version": "3.4.1", "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-3.4.1.tgz", @@ -2632,8 +2640,7 @@ "iconv-lite": { "version": "0.4.18", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.18.tgz", - "integrity": "sha512-sr1ZQph3UwHTR0XftSbK85OvBbxe/abLGzEnPENCQwmHf7sck8Oyu4ob3LgBxWWxRoM+QszeUyl7jbqapu2TqA==", - "dev": true + "integrity": "sha512-sr1ZQph3UwHTR0XftSbK85OvBbxe/abLGzEnPENCQwmHf7sck8Oyu4ob3LgBxWWxRoM+QszeUyl7jbqapu2TqA==" }, "icss-replace-symbols": { "version": "1.1.0", @@ -3104,8 +3111,7 @@ "is-stream": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-1.1.0.tgz", - "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=", - "dev": true + "integrity": "sha1-EtSj3U5o4Lec6428hBc66A2RykQ=" }, "is-svg": { "version": "2.1.0", @@ -3153,6 +3159,15 @@ "dev": true, "requires": { "isarray": "1.0.0" + } + }, + "isomorphic-unfetch": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/isomorphic-unfetch/-/isomorphic-unfetch-2.0.0.tgz", + "integrity": "sha1-9QFApMFj11grXzfxWRloxPgJpkU=", + "requires": { + "node-fetch": "1.7.3", + "unfetch": "3.0.0" } }, "jest-docblock": { @@ -3962,6 +3977,15 @@ "isarray": "0.0.1" } } + } + }, + "node-fetch": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-1.7.3.tgz", + "integrity": "sha512-NhZ4CsKx7cYm2vSrBAr2PvFOe6sWDf0UYLRqA6svUYg7+/TSfVAu49jYC4BvQ4Sms9SZgdqGBgroqfDhJdTyKQ==", + "requires": { + "encoding": "0.1.12", + "is-stream": "1.1.0" } }, "node-forge": { @@ -6501,6 +6525,11 @@ "integrity": "sha1-7Mo6A+VrmvFzhbqsgSrIO5lKli8=", "dev": true }, + "unfetch": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/unfetch/-/unfetch-3.0.0.tgz", + "integrity": "sha1-jR4FE6Ts0OX/LUGmund3Gq6LZII=" + }, "uniq": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/uniq/-/uniq-1.0.1.tgz", diff --git a/package.json b/package.json index 4d14a14..75b7ab7 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ }, "dependencies": { "express": "^4.15.3", + "isomorphic-unfetch": "^2.0.0", "path-to-regexp": "^2.0.0", "preact": "^8.2.5", "preact-render-to-string": "^3.6.3" diff --git a/src/client/tsconfig.json b/src/client/tsconfig.json index 89f6fd7..27816ce 100644 --- a/src/client/tsconfig.json +++ b/src/client/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../tsconfig.json", "include": [ - "." + "../common", "." ], "compilerOptions": { "target": "ES5", diff --git a/src/common/components/app/app.tsx b/src/common/components/app/app.tsx index df006bd..b94d5a3 100644 --- a/src/common/components/app/app.tsx +++ b/src/common/components/app/app.tsx @@ -1,9 +1,10 @@ +import { h } from "preact"; import "./app.css"; -import { ComponentProps, h } from "preact"; import { about, index, wiki } from "../../../common/routers/api"; +import { ChildrenProps } from "../preact-utils"; import Link from "../link"; -export default function App({ children }: ComponentProps<any>): JSX.Element { +export default function App({ children }: ChildrenProps): JSX.Element { return ( <div class="App"> <ul> @@ -17,9 +18,17 @@ <Link href={wiki.url({ title: "Banana" })}>Banana</Link> </li> <li> + <Link href={wiki.url({ title: "Bill_&_Ted's_Excellent_Adventure" })}> + Bill & Ted's Excellent Adventure + </Link> + </li> + <li> <Link href={wiki.url({ title: "Cucumber" })}>Cucumber</Link> </li> <li> + <Link href={wiki.url({ title: "Ice_cream" })}>Ice cream</Link> + </li> + <li> <Link href="/404">404</Link> </li> </ul> diff --git a/src/common/components/link.tsx b/src/common/components/link.tsx index 60dc612..c815a9d 100644 --- a/src/common/components/link.tsx +++ b/src/common/components/link.tsx @@ -1,7 +1,8 @@ -import { ComponentProps, h } from "preact"; +import { h } from "preact"; import { History } from "history"; +import { ChildrenProps, classOf, ClassProps } from "./preact-utils"; -export interface Props extends ComponentProps<any> { +export interface Props extends ClassProps, ChildrenProps { href: string; } @@ -10,18 +11,18 @@ * control to History. All local hyperlinks should use a Link component. */ export default function Link( - props: Props, + { href, children, ...props }: Props, context: { history?: History } ): JSX.Element { - const { children, ...anchorProps } = props; return ( <a - {...anchorProps} + class={classOf("Link", props.class)} + href={href} onClick={event => { // todo: check that link is internal (relative). if (context.history) { event.preventDefault(); - context.history.push(anchorProps.href); + context.history.push(href); } }} > diff --git a/src/common/components/page-summary/page-summary.css b/src/common/components/page-summary/page-summary.css new file mode 100644 index 0000000..fb07c7c --- /dev/null +++ b/src/common/components/page-summary/page-summary.css @@ -0,0 +1,24 @@ +.PageSummary { +} + +.PageSummary-header {} +.PageSummary-title {} +.PageSummary-description {} +.PageSummary-description::first-letter { + text-transform: uppercase; +} + +.PageSummary-body { +} +.PageSummary-thumbnail { + float: left; + margin: 0 var(--space) var(--space) 0; +} +.PageSummary-extract { +} + +.PageSummary-footer { + clear: both; +} +.PageSummary-lastModified { +} diff --git a/src/common/components/page-summary/page-summary.tsx b/src/common/components/page-summary/page-summary.tsx new file mode 100644 index 0000000..42ce83d --- /dev/null +++ b/src/common/components/page-summary/page-summary.tsx @@ -0,0 +1,46 @@ +import { h } from "preact"; +import "./page-summary.css"; +import { PageSummary as PageSummaryModel } from "../../models/page"; + +export interface Props { + summary: PageSummaryModel; +} + +export const PageSummary = ({ summary }: Props): JSX.Element => ( + <div class="PageSummary"> + <div class="PageSummary-header"> + <h2 + class="PageSummary-title" + dangerouslySetInnerHTML={{ __html: summary.titleHTML }} + /> + {summary.descriptionText && ( + <p class="PageSummary-description">{summary.descriptionText}</p> + )} + </div> + <div class="PageSummary-body"> + {summary.thumbnail && + summary.image && ( + /* todo: replace with Link. */ + <a href={summary.image.url}> + <img + class="PageSummary-thumbnail" + src={summary.thumbnail.url} + width={summary.thumbnail.width} + height={summary.thumbnail.height} + /> + </a> + )} + {summary.extractHTML && ( + <p + class="PageSummary-extract" + dangerouslySetInnerHTML={{ __html: summary.extractHTML }} + /> + )} + </div> + <div class="PageSummary-footer"> + <p class="PageSummary-lastModified"> + {summary.lastModified.toLocaleString()} + </p> + </div> + </div> +); diff --git a/src/common/components/paper/paper.tsx b/src/common/components/paper/paper.tsx index 6e8ba60..daf9249 100644 --- a/src/common/components/paper/paper.tsx +++ b/src/common/components/paper/paper.tsx @@ -1,6 +1,7 @@ +import { h } from "preact"; +import { ChildrenProps } from "../preact-utils"; import "./paper.css"; -import { ComponentProps, h } from "preact"; -export default function Paper({ children }: ComponentProps<any>): JSX.Element { +export default function Paper({ children }: ChildrenProps): JSX.Element { return <div class="Paper">{children}</div>; } diff --git a/src/common/components/preact-utils.ts b/src/common/components/preact-utils.ts new file mode 100644 index 0000000..256a2d5 --- /dev/null +++ b/src/common/components/preact-utils.ts @@ -0,0 +1,14 @@ +// todo: delete pending https://github.com/developit/preact/pull/869. +export type ComponentChild = JSX.Element | string; +export type ComponentChildren = ComponentChild[]; + +export interface ChildrenProps { + children?: ComponentChildren; +} + +export interface ClassProps { + class?: string; +} + +export const classOf = (...names: (string | undefined)[]): string => + names.filter(name => name).join(" "); diff --git a/src/common/components/with-context.ts b/src/common/components/with-context.ts index 53e594c..4c35ce6 100644 --- a/src/common/components/with-context.ts +++ b/src/common/components/with-context.ts @@ -3,6 +3,8 @@ export interface Props { history: History; + // todo: replace with ChildrenProps when + // https://github.com/developit/preact/pull/869 is merged. children?: JSX.Element[]; } diff --git a/src/common/data-clients/page-data-client.ts b/src/common/data-clients/page-data-client.ts new file mode 100644 index 0000000..5a914a6 --- /dev/null +++ b/src/common/data-clients/page-data-client.ts @@ -0,0 +1,20 @@ +import * as fetch from "isomorphic-unfetch"; +import { PageSummary, PageTitlePath } from "../models/page"; +import { unmarshalPageSummary } from "../marshallers/page-unmarshaller"; + +export interface Params { + titlePath: PageTitlePath; +} + +const url = ({ titlePath }: Params) => + `https://en.wikipedia.org/api/rest_v1/page/summary/${titlePath}`; + +const HEADERS = { + accept: + 'application/json; charset=utf-8; profile="https://www.mediawiki.org/wiki/Specs/Summary/1.2.0"' +}; + +export const requestPageSummary = (params: Params): Promise<PageSummary> => + fetch(url(params), { headers: HEADERS }) + .then((response: Response) => response.json()) + .then(unmarshalPageSummary); diff --git a/src/common/models/page.ts b/src/common/models/page.ts index daf22c9..331c10b 100644 --- a/src/common/models/page.ts +++ b/src/common/models/page.ts @@ -1,3 +1,22 @@ +/** + * URL-decoded normalized wiki URL path. e.g.: Main_Page, + * Bill_&_Ted's_Excellent_Adventure. + */ +export type PageTitleID = string; + +/** + * URL-encoded normalized wiki URL path. e.g.: Main_Page, + * Bill_%26_Ted%27s_Excellent_Adventure. + */ +export type PageTitlePath = string; + +/** + * Plain text localized page title. e.g.: + * Possible: Banana, Main Page, Bill & Ted's Excellent Adventure, Talk:Pie. + * Impossible: Main_Page, Bill_%26_Ted%27s_Excellent_Adventure. + */ +export type PageTitleText = string; + export interface PageThumbnail { url: string; originalURL: string; @@ -21,7 +40,7 @@ localeDirection: string; pageID: number; lastModified: Date; - titleText: string; + titleText: PageTitleText; titleHTML: string; descriptionText: string; extractText: string; diff --git a/src/common/pages/wiki.tsx b/src/common/pages/wiki.tsx index 995f443..80eeabf 100644 --- a/src/common/pages/wiki.tsx +++ b/src/common/pages/wiki.tsx @@ -1,21 +1,34 @@ -import { ComponentProps, h } from "preact"; +import { h } from "preact"; import App from "../components/app/app"; -import { RouteParameters } from "../routers/route"; +import { PageSummary } from "../components/page-summary/page-summary"; +import { + PageSummary as PageSummaryModel, + PageTitleID, + PageTitlePath +} from "../models/page"; +import Paper from "../components/paper/paper"; +import { RouteParams } from "../routers/route"; +import { requestPageSummary } from "../data-clients/page-data-client"; -export interface Parameters extends RouteParameters { - title: string; +export interface Params extends RouteParams { + /** + * When used as an input, an unencoded PageTitleID; when used as an output, + * an encoded PageTitlePath. + */ + title: PageTitleID | PageTitlePath; } -export interface Props extends ComponentProps<any> { - title: string; +export interface Props { + summary: PageSummaryModel; } -export const initialProps = ({ title }: Parameters): Promise<Props> => { - return Promise.resolve({ title }); -}; +export const initialProps = ({ title }: Params): Promise<Props> => + requestPageSummary({ titlePath: title }).then(summary => ({ summary })); -export const Component = ({ title }: Props): JSX.Element => ( +export const Component = ({ summary }: Props): JSX.Element => ( <App> - <p>{title}</p> + <Paper> + <PageSummary summary={summary} /> + </Paper> </App> ); diff --git a/src/common/routers/api.ts b/src/common/routers/api.ts index 9ea04e8..c45a584 100644 --- a/src/common/routers/api.ts +++ b/src/common/routers/api.ts @@ -1,4 +1,4 @@ -import { Props as WikiProps } from "../pages/wiki"; +import { Props as WikiProps, Params as WikiParams } from "../pages/wiki"; import { AnyRoute, Route, newRoute } from "./route"; export const index: Route = newRoute({ @@ -15,7 +15,7 @@ chunkName: "pages/about" }); -export const wiki: Route<WikiProps, void, { title: string }> = newRoute({ +export const wiki: Route<WikiProps, void, WikiParams> = newRoute({ path: "/wiki/:title", endpoint: () => import(/* webpackChunkName: "pages/wiki" */ "../pages/wiki"), chunkName: "pages/wiki" diff --git a/src/common/routers/route.ts b/src/common/routers/route.ts index af75f4b..cbd5cb3 100644 --- a/src/common/routers/route.ts +++ b/src/common/routers/route.ts @@ -1,7 +1,7 @@ import * as pathToRegExp from "path-to-regexp"; import { AnyComponent } from "preact"; -export interface RouteParameters { +export interface RouteParams { [name: string]: string; } @@ -13,7 +13,7 @@ * A function that returns a Promise for the dependencies needed to construct * the view component such as a remote resource. */ - initialProps?: (parameters: RouteParameters) => Promise<Props>; + initialProps?: (parameters: RouteParams) => Promise<Props>; } export interface RouteConfiguration<Props = void, State = void> { @@ -28,7 +28,7 @@ status: number; /** Generates a URL from parameters. */ - url: (properties?: Parameters) => string; + url: (parameters?: Parameters) => string; } export type AnyRoute = Route<any, any, any>; diff --git a/src/common/routers/router.ts b/src/common/routers/router.ts index 3e1708e..0795667 100644 --- a/src/common/routers/router.ts +++ b/src/common/routers/router.ts @@ -1,10 +1,6 @@ import * as pathToRegExp from "path-to-regexp"; import { AnyComponent } from "preact"; -import { - AnyRoute, - Endpoint, - RouteParameters -} from "../../common/routers/route"; +import { AnyRoute, Endpoint, RouteParams } from "../../common/routers/route"; export interface RouteResponse<Props, State> { chunkName: string; @@ -40,10 +36,10 @@ const newRouteParameters = ( parameterNames: pathToRegExp.Key[], parameterValues: string[] -): RouteParameters => +): RouteParams => parameterNames.reduce( ( - parameters: RouteParameters, + parameters: RouteParams, parameterName: pathToRegExp.Key, index: number ) => { @@ -55,7 +51,7 @@ function requestInitialProps<Props>( endpoint: Endpoint<Props, any>, - parameters: RouteParameters + parameters: RouteParams ): Promise<Props | {}> { if (endpoint.initialProps) { return endpoint.initialProps(parameters); @@ -66,7 +62,7 @@ const respond = ( route: ParsedRoute, url: string, - parameters: RouteParameters + parameters: RouteParams ): Promise<RouteResponse<any, any>> => route.endpoint().then((endpoint: Endpoint<any, any>) => requestInitialProps(endpoint, parameters).then((props: any) => ({ diff --git a/src/common/types/isomorphic-unfetch.d.ts b/src/common/types/isomorphic-unfetch.d.ts new file mode 100644 index 0000000..7dd0676 --- /dev/null +++ b/src/common/types/isomorphic-unfetch.d.ts @@ -0,0 +1,5 @@ +declare const unfetch: typeof fetch; + +declare module "isomorphic-unfetch" { + export = unfetch; +} diff --git a/src/common/types/preact.d.ts b/src/common/types/preact.d.ts deleted file mode 100644 index 7ac7b19..0000000 --- a/src/common/types/preact.d.ts +++ /dev/null @@ -1,2 +0,0 @@ -// Delete pending https://github.com/developit/preact/pull/869. -export type Children = (JSX.Element | JSX.Element[] | string)[]; diff --git a/src/server/components/page.tsx b/src/server/components/page.tsx index fd8bbdb..161d940 100644 --- a/src/server/components/page.tsx +++ b/src/server/components/page.tsx @@ -1,14 +1,12 @@ import { h } from "preact"; import { Manifest, asset, scripts, style } from "../assets/manifest"; -import { Children } from "../../common/types/preact"; +import { ChildrenProps } from "../../common/components/preact-utils"; -export interface Props { +export interface Props extends ChildrenProps { // Title of the page title: string; manifest: Manifest; chunkName: string; - // HTML to render in the body of the page - children?: Children; } export function Page({ @@ -24,7 +22,7 @@ <head> <meta charSet="utf-8" /> <meta http-equiv="x-ua-compatible" content="ie=edge" /> - <meta name="viewport" content="width=device-width,initial-scale=1" /> + <meta name="viewport" content="width=device-width, initial-scale=1" /> <title>{title ? `${title} - ` : ""}Marvin</title> <base href="/" /> <link rel="stylesheet" href={style(manifest)} /> diff --git a/src/server/index.tsx b/src/server/index.tsx index 35791d7..ede6d5d 100644 --- a/src/server/index.tsx +++ b/src/server/index.tsx @@ -25,12 +25,12 @@ server.use("/public", express.static("dist/public")); -const render = ({ chunkName, Component }: RouteResponse<any, any>) => { +const render = ({ chunkName, Component, props }: RouteResponse<any, any>) => { return ( "<!doctype html>" + // eslint-disable-line prefer-template renderToString( <Page title="" manifest={manifest} chunkName={chunkName}> - <Component /> + <Component {...props} /> </Page> ) ); diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json index 423de29..0bd1839 100644 --- a/src/server/tsconfig.json +++ b/src/server/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../tsconfig.json", "include": [ - "." + "../common", "." ], "compilerOptions": { "target": "ES2015", diff --git a/webpack.config.ts b/webpack.config.ts index cc1602c..3c1adc8 100644 --- a/webpack.config.ts +++ b/webpack.config.ts @@ -57,10 +57,10 @@ // each entry which breaks caching. This chunk changes whenever any code // anywhere changes. - // Client package dependencies (these should match package.json's + // Client package dependencies (these should be a subset of package.json's // `dependencies`). This chunk changes when one of the specified // dependencies changes. - vendor: ["history", "path-to-regexp", "preact"] + vendor: ["history", "isomorphic-unfetch", "path-to-regexp", "preact"] }, stats: STATS, @@ -187,6 +187,8 @@ new webpack.NamedModulesPlugin(), new ExtractTextPlugin({ + // `contenthash` is not actually a chunk hash: + // https://github.com/webpack-contrib/extract-text-webpack-plugin/issues/504#issuecomment-306581954. filename: PRODUCTION ? "[name].[contenthash].css" : "[name].css" }), @@ -209,7 +211,8 @@ // during execution for module resolution, dynamic importing, and more. // Without this distinct runtime chunk, it's instead bundled into each entry, // including vendor, which breaks caching. This chunk changes whenever any - // other file changes. + // other file changes. See + // https://webpack.js.org/plugins/commons-chunk-plugin/#manifest-file. new webpack.optimize.CommonsChunkPlugin({ // This name should NOT match any `configuration.entry`. name: "runtime" -- To view, visit https://gerrit.wikimedia.org/r/378267 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7456e654e24db34343bd8e108bc990be8fff31fe Gerrit-PatchSet: 9 Gerrit-Project: marvin Gerrit-Branch: master Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: Jhernandez <jhernan...@wikimedia.org> Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: Sniedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits