jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/392429 )
Change subject: Fix: don't reference the manifest in the client
......................................................................
Fix: don't reference the manifest in the client
The client cannot access the asset manifest because its generated from
the client itself. Move manifest read logic back to the server but keep
the asset path generation logic in common. This patch fixes a broken
manifest require reference in the client and unintentional modification
of assets#scripts in the server.
Bug: T180893
Change-Id: Iebb827f597575932e87dd1f0e23673a92eae7f7b
---
M src/common/assets/manifest.ts
M src/server/components/html-page.tsx
2 files changed, 36 insertions(+), 48 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 7e1cd18..85964b2 100644
--- a/src/common/assets/manifest.ts
+++ b/src/common/assets/manifest.ts
@@ -1,52 +1,23 @@
-import { PRODUCTION, WEBPACK_DEV_SERVER_URL } from "./config";
-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;
+import { Assets } from "assets-webpack-plugin";
+import { PRODUCTION, WEBPACK_DEV_SERVER_URL } from
"../../common/assets/config";
/**
* @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(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
- return `${manifest}/public/${entry}.${extension}`;
- else if (manifest[entry] && manifest[entry][extension])
- // When it is an object, if the entry exists, just return its path
+export function asset(
+ entry: string,
+ extension: string,
+ manifest?: Assets
+): string {
+ if (manifest && manifest[entry] && manifest[entry][extension])
+ // When the manifest is present and the entry exists, just return its path.
return manifest[entry][extension];
- else
- // If the entry is not on the asset manifest, then just point to it
directly
- // to the static assets path (copied there as-is from src/public)
- return `/public/${entry}.${extension}`;
+
+ // When the manifest or entry doesn't exist, use the public path which points
+ // to a static file. For prod, files are copied from src/public. For dev,
+ // all assets are unhashed and served from /public/.
+ const baseURL = PRODUCTION ? "" : WEBPACK_DEV_SERVER_URL;
+ return `${baseURL}/public/${entry}.${extension}`;
}
-
-// Note: scripts must be included in the correct order: runtime, vendor, index.
-// Example errors:
-// - No runtime:
-//
-// vendor.js:1 Uncaught ReferenceError: webpackJsonp is not defined
-// at vendor.js:1
-//
-// - No vendor:
-//
-// Uncaught TypeError: Cannot read property 'call' of undefined
-// at __webpack_require__ (runtime.js:55)
-// at Object.0 (index.js:204)
-// at __webpack_require__ (runtime.js:55)
-// at webpackJsonpCallback (runtime.js:26)
-// at index.js:1
-
-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 const style: string = asset("index", "css");
diff --git a/src/server/components/html-page.tsx
b/src/server/components/html-page.tsx
index 12cd9b4..8ebb50e 100644
--- a/src/server/components/html-page.tsx
+++ b/src/server/components/html-page.tsx
@@ -1,6 +1,8 @@
import { h } from "preact";
-import { asset, scripts, style } from "../../common/assets/manifest";
+import { PRODUCTION } from "../../common/assets/config";
+import { asset } from "../../common/assets/manifest";
import { ChildrenProps } from "../../common/components/preact-utils";
+declare function __non_webpack_require__(name: string): any; //
eslint-disable-line camelcase
export interface Props extends ChildrenProps {
// Title of the page
@@ -8,13 +10,28 @@
chunkName: 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")
+ : undefined;
+
export default function HTMLPage({
title = "",
chunkName,
children
}: Props): JSX.Element {
- const assets: string[] = scripts;
- assets.push(asset(chunkName, "js"));
+ // Asset order matters.
+ const assets = [
+ asset("runtime", "js", manifest),
+ asset("vendor", "js", manifest),
+ asset("index", "js", manifest),
+ asset(chunkName, "js", manifest)
+ ];
+ const style = asset("index", "css", manifest);
+ const favicon = asset("favicon/wikipedia", "ico", manifest);
return (
<html lang="en">
@@ -27,7 +44,7 @@
<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={asset("favicon/wikipedia", "ico")} />
+ <link rel="shortcut icon" href={favicon} />
</head>
<body>
<div id="root">{children}</div>
--
To view, visit https://gerrit.wikimedia.org/r/392429
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iebb827f597575932e87dd1f0e23673a92eae7f7b
Gerrit-PatchSet: 1
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: Jhernandez <jhernan...@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
