jenkins-bot has submitted this change and it was merged. (
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
M test/server/components/html-page.test.ts
4 files changed, 24 insertions(+), 54 deletions(-)
Approvals:
Jhernandez: Looks good to me, approved
jenkins-bot: Verified
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>
)
diff --git a/test/server/components/html-page.test.ts
b/test/server/components/html-page.test.ts
index 147466b..a9b1243 100644
--- a/test/server/components/html-page.test.ts
+++ b/test/server/components/html-page.test.ts
@@ -6,7 +6,6 @@
it("contains a root div with the children when rendered", () => {
const vNode = HTMLPage({
title: "Test",
- manifest: "",
chunkName: "",
children: ["body"]
});
@@ -19,7 +18,7 @@
});
it("contains a <title/> when rendered", () => {
- const vNode = HTMLPage({ title: "Test", manifest: "", chunkName: "" });
+ const vNode = HTMLPage({ title: "Test", chunkName: "" });
const html = render(vNode);
const expected = "<title>Test - Marvin</title>";
assert.ok(
--
To view, visit https://gerrit.wikimedia.org/r/391898
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic1871e5db1106bb513eaa0cf78c2b3bd7940be52
Gerrit-PatchSet: 2
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Jhernandez <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
