Niedzielski has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/391898 )

Change subject: Chore: consolidate manifest logic
......................................................................

Chore: consolidate manifest logic

Move all manifest logic into manifest.ts since these are known at
compile time and needed by both client and server.

Bug: T180623
Change-Id: Ic1871e5db1106bb513eaa0cf78c2b3bd7940be52
---
M src/common/assets/manifest.ts
M src/server/components/html-page.tsx
M src/server/index.tsx
3 files changed, 23 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/marvin refs/changes/98/391898/1

diff --git a/src/common/assets/manifest.ts b/src/common/assets/manifest.ts
index b1097f9..7e1cd18 100644
--- a/src/common/assets/manifest.ts
+++ b/src/common/assets/manifest.ts
@@ -1,18 +1,20 @@
-import { Assets } from "assets-webpack-plugin";
+import { PRODUCTION, WEBPACK_DEV_SERVER_URL } from "./config";
+declare function __non_webpack_require__(name: string): any; // 
eslint-disable-line camelcase
 
-/** Manifest of filename entry points to bundled asset paths. */
-export type Manifest = Assets | string;
+// The production asset manifest from the public build products or
+// the webpack-dev-server URL (which has no manifest). The former doesn't exist
+// at compilation time, so use a dynamic require to read it from the filesystem
+// at run time in production builds.
+const manifest = PRODUCTION
+  ? __non_webpack_require__("../public/assets-manifest.json")
+  : WEBPACK_DEV_SERVER_URL;
 
 /**
  * @return The path to the asset identified by entry and extension (e.g.,
  *         index.js); either a URL (development) or a filesystem path
  *         (production).
  */
-export function asset(
-  manifest: Manifest,
-  entry: string,
-  extension: string
-): string {
+export function asset(entry: string, extension: string): string {
   if (typeof manifest === "string")
     // When the manifest is a string, it is the URL of something like
     // webpack-dev-server, so just point to there for the asset
@@ -42,22 +44,9 @@
 //       at webpackJsonpCallback (runtime.js:26)
 //       at index.js:1
 
-export function runtime(manifest: Manifest): string {
-  return asset(manifest, "runtime", "js");
-}
+export const runtime: string = asset("runtime", "js");
+export const vendor: string = asset("vendor", "js");
+export const index: string = asset("index", "js");
+export const scripts: string[] = [runtime, vendor, index];
 
-export function vendor(manifest: Manifest): string {
-  return asset(manifest, "vendor", "js");
-}
-
-export function index(manifest: Manifest): string {
-  return asset(manifest, "index", "js");
-}
-
-export function scripts(manifest: Manifest): string[] {
-  return [runtime(manifest), vendor(manifest), index(manifest)];
-}
-
-export function style(manifest: Manifest): string {
-  return asset(manifest, "index", "css");
-}
+export const style: string = asset("index", "css");
diff --git a/src/server/components/html-page.tsx 
b/src/server/components/html-page.tsx
index 05d9777..12cd9b4 100644
--- a/src/server/components/html-page.tsx
+++ b/src/server/components/html-page.tsx
@@ -1,24 +1,20 @@
 import { h } from "preact";
-import { Manifest, asset, scripts, style } from "../../common/assets/manifest";
+import { asset, scripts, style } from "../../common/assets/manifest";
 import { ChildrenProps } from "../../common/components/preact-utils";
 
 export interface Props extends ChildrenProps {
   // Title of the page
   title: string;
-  manifest: Manifest;
   chunkName: string;
 }
 
 export default function HTMLPage({
   title = "",
-  manifest,
   chunkName,
   children
 }: Props): JSX.Element {
-  const assets: string[] = scripts(manifest);
-  assets.push(asset(manifest, chunkName, "js"));
-
-  const favicon = asset(manifest, "favicon/wikipedia", "ico");
+  const assets: string[] = scripts;
+  assets.push(asset(chunkName, "js"));
 
   return (
     <html lang="en">
@@ -28,10 +24,10 @@
         <meta name="viewport" content="width=device-width, initial-scale=1" />
         <title>{title ? `${title} - ` : ""}Marvin</title>
         {/* Preload the stylesheet before the scripts */}
-        <link rel="preload" href={style(manifest)} as="style" />
-        <link rel="stylesheet" href={style(manifest)} />
+        <link rel="preload" href={style} as="style" />
+        <link rel="stylesheet" href={style} />
         {assets.map(path => <link rel="preload" href={path} as="script" />)}
-        <link rel="shortcut icon" href={favicon} />
+        <link rel="shortcut icon" href={asset("favicon/wikipedia", "ico")} />
       </head>
       <body>
         <div id="root">{children}</div>
diff --git a/src/server/index.tsx b/src/server/index.tsx
index 9c166cc..3e46ff2 100644
--- a/src/server/index.tsx
+++ b/src/server/index.tsx
@@ -5,22 +5,8 @@
 import { RouteResponse, newRouter } from "../common/routers/router";
 import { RedirectError } from "../common/http/fetch-with-redirect";
 import { routes } from "../common/routers/api";
-import {
-  PRODUCTION,
-  SERVER_PORT,
-  SERVER_URL,
-  WEBPACK_DEV_SERVER_URL
-} from "../common/assets/config";
+import { SERVER_PORT, SERVER_URL } from "../common/assets/config";
 import HTMLPage from "./components/html-page";
-declare function __non_webpack_require__(name: string): any; // 
eslint-disable-line camelcase
-
-// The production asset manifest from the public build products or
-// the webpack-dev-server URL (which has no manifest). The former doesn't exist
-// at compilation time, so use a dynamic require to read it from the filesystem
-// at run time in production builds.
-const manifest = PRODUCTION
-  ? __non_webpack_require__("../public/assets-manifest.json")
-  : WEBPACK_DEV_SERVER_URL;
 
 const server = express();
 
@@ -32,7 +18,7 @@
   return (
     "<!doctype html>" + // eslint-disable-line prefer-template
     renderToString(
-      <HTMLPage title="" manifest={manifest} chunkName={chunkName}>
+      <HTMLPage title="" chunkName={chunkName}>
         <Component {...props} />
       </HTMLPage>
     )

-- 
To view, visit https://gerrit.wikimedia.org/r/391898
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic1871e5db1106bb513eaa0cf78c2b3bd7940be52
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

Reply via email to