Replace Next.js' static assetPrefix with __webpack_public_path__ …

…to support dynamic path prefix (#16967) ## Summary & Motivation This pr addresses a performance issue reported by a user: https://dagster.slack.com/archives/C01U954MEER/p1695936650103649 This is an alternative approach to #16941 that avoids needing to string replace javascript files at run time and instead replaces at buildtime using `__webpack_public_path__`. This solution is pretty hacky for webworkers and may need tweaking in the future if we start SSRing more of the app. For workers we grab the path from the global location object. ## How I Tested These Changes Built the app and ran `dagster-webserver -p 4444 --path-prefix=/test1235` and made sure the app still worked including webworkers. This doesn't affect cloud or local development (I tested local development and it just ignores __webpack_public_path__).
dagster-io · Oct 3, 2023 · acd3bf4 · acd3bf4
1 parent dc9919c
commit acd3bf4
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 21 deletions.
diff --git a/js_modules/dagster-ui/package.json b/js_modules/dagster-ui/package.json
@@ -3,7 +3,8 @@
   "version": "0.1.0",
   "private": true,
   "scripts": {
-    "build": "yarn workspace @dagster-io/app-oss build && yarn post-build",
+    "build": "yarn workspace @dagster-io/app-oss build && yarn replace-asset-prefix && yarn post-build",
+    "replace-asset-prefix": "node ./packages/app-oss/replace-asset-prefix.js",
     "build-with-profiling": "yarn workspace @dagster-io/app-oss build --profile && yarn post-build",
     "post-build": "cd ../../python_modules/dagster-webserver/dagster_webserver && rm -rf webapp && mkdir -p webapp && cp -r ../../../js_modules/dagster-ui/packages/app-oss/build ./webapp/ && mkdir -p webapp/build/vendor && cp -r graphql-playground ./webapp/build/vendor && cp ../../../js_modules/dagster-ui/packages/app-oss/csp-header.txt ./webapp/build",
     "lint": "yarn workspace @dagster-io/app-oss lint && yarn workspace @dagster-io/ui-core lint && yarn workspace @dagster-io/ui-components lint",

diff --git a/js_modules/dagster-ui/packages/app-oss/replace-asset-prefix.js b/js_modules/dagster-ui/packages/app-oss/replace-asset-prefix.js
@@ -0,0 +1,34 @@
+/* eslint-disable @typescript-eslint/no-var-requires */
+const fs = require('fs');
+const path = require('path');
+
+function replaceStringInFile(filePath, searchString, replacement) {
+  const fileContent = fs.readFileSync(filePath, 'utf8');
+  const replacedContent = fileContent.replace(new RegExp(searchString, 'g'), replacement);
+  fs.writeFileSync(filePath, replacedContent);
+}
+
+function processDirectory(directory) {
+  const files = fs.readdirSync(directory);
+
+  for (const file of files) {
+    const filePath = path.join(directory, file);
+
+    const isDirectory = fs.statSync(filePath).isDirectory();
+
+    if (isDirectory) {
+      // Recursively process subdirectories
+      processDirectory(filePath);
+    } else if (file.endsWith('.js')) {
+      replaceStringInFile(
+        filePath,
+        '"BUILDTIME_ASSETPREFIX_REPLACE_ME',
+        // if typeof window === "undefined" then we are inside a web worker
+        // Grab the path from the location object
+        '(() => {if (typeof window === "undefined") {return self.location.pathname.split("/_next/")[0]} return self.__webpack_public_path__ || "";})() + "',
+      );
+    }
+  }
+}
+
+processDirectory(__dirname + '/build/');
diff --git a/js_modules/dagster-ui/packages/app-oss/src/pages/_document.tsx b/js_modules/dagster-ui/packages/app-oss/src/pages/_document.tsx
@@ -14,6 +14,12 @@ export default function Document() {
   return (
     <Html lang="en">
       <Head nonce="NONCE-PLACEHOLDER">
+        <script
+          nonce="NONCE-PLACEHOLDER"
+          dangerouslySetInnerHTML={{
+            __html: 'window.__webpack_public_path__ = "__PATH_PREFIX__"',
+          }}
+        />
         {/* Not sure if we need the following script */}
         <script
           id="webpack-nonce-setter"

diff --git a/python_modules/dagster-webserver/dagster_webserver/webserver.py b/python_modules/dagster-webserver/dagster_webserver/webserver.py
@@ -27,9 +27,7 @@
     JSONResponse,
     PlainTextResponse,
     RedirectResponse,
-    Response,
     StreamingResponse,
-    guess_type,
 )
 from starlette.routing import Mount, Route, WebSocketRoute
 from starlette.types import Message
@@ -231,16 +229,6 @@ def index_html_endpoint(self, request: Request):
                 """)
 
     def build_static_routes(self):
-        def next_file_response(file_path):
-            with open(file_path, encoding="utf8") as f:
-                content = f.read().replace(
-                    "BUILDTIME_ASSETPREFIX_REPLACE_ME", f"{self._app_path_prefix}"
-                )
-                return Response(content=content, media_type=guess_type(file_path)[0])
-
-        def _next_static_file(path, file_path):
-            return Route(path, lambda _: next_file_response(file_path), name="next_static")
-
         def _static_file(path, file_path):
             return Route(
                 path,
@@ -253,16 +241,9 @@ def _static_file(path, file_path):
         for subdir, _, files in walk(base_dir):
             for file in files:
                 full_path = path.join(subdir, file)
-
                 # Replace path.sep to make sure our routes use forward slashes on windows
                 mount_path = "/" + full_path[len(base_dir) :].replace(path.sep, "/")
-                # We only need to replace BUILDTIME_ASSETPREFIX_REPLACE_ME in javascript files
-                if self._uses_app_path_prefix and (
-                    file.endswith(".js") or file.endswith(".js.map")
-                ):
-                    routes.append(_next_static_file(mount_path, full_path))
-                else:
-                    routes.append(_static_file(mount_path, full_path))
+                routes.append(_static_file(mount_path, full_path))
 
         # No build directory, this happens in a test environment. Don't fail loudly since we already have other tests that will fail loudly if
         # there is in fact no build