restore defaults

Author: tido64

.changeset/busy-ads-rescue.md

@@ -3,4 +3,18 @@
 "@rnx-kit/metro-service": patch
 ---
 
-Fixed assets outside of project root not rendering
+Fixed assets outside of project root not rendering. This breaks release builds
+so it must be manually enabled in `metro.config.js`:
+
+```diff
+diff --git a/metro.config.js b/metro.config.js
+index 000000000..000000000 100644
+--- a/metro.config.js
++++ b/metro.config.js
+@@ -34,4 +34,5 @@ module.exports = makeMetroConfig({
+     blacklistRE: blockList,
+     blockList,
+   },
++  unstable_allowAssetsOutsideProjectRoot: true,
+ });
+```

packages/metro-config/src/assetPlugins/escapeAssetURLs.js

@@ -1,50 +1,46 @@
 /**
  * @typedef {import("metro").AssetData} AssetData;
- * @typedef {import("metro-config").ConfigT} ConfigT;
- * @typedef {import("metro-config").Middleware} Middleware;
- *
- * @typedef {import("metro/private/Server").default & {
- *   _config?: ConfigT;
- * }} Server;
  */
 
+/**
+ * @param {string} str
+ * @param {string} searchValue
+ * @param {string} replaceValue
+ * @returns {string}
+ */
+function replaceString(str, searchValue, replaceValue) {
+  return str.startsWith("/assets/")
+    ? str.replaceAll(searchValue, replaceValue)
+    : str;
+}
+
 /**
  * Metro doesn't support assets in a monorepo setup. When the app requests
  * assets at URLs such as `/assets/../../../node_modules/react-native/<...>`,
  * the URL will be resolved to `/node_modules/react-native/<...>` and Metro will
  * fail to resolve them. The workaround is to replace `..` with something else
  * so the URL doesn't collapse when resolved, then restore them in
- * `server.enhanceMiddleware`.
+ * `server.rewriteRequestUrl`.
  *
  * For more details, see https://github.com/facebook/metro/issues/290.
  *
  * @param {import("type-fest").Writable<AssetData>} assetData
  * @returns {AssetData}
  */
 function assetPlugin(assetData) {
-  assetData.httpServerLocation = assetData.httpServerLocation.replaceAll(
-    "../",
-    "@@/"
-  );
+  const url = assetData.httpServerLocation;
+  assetData.httpServerLocation = replaceString(url, "../", "@@/");
   return assetData;
 }
 
 /**
- * This middleware restores `..` in asset URLs.
- *
- * @param {Middleware} middleware
- * @param {Server} _server
- * @returns {import("connect").NextHandleFunction}
+ * Restores `..` in asset URLs.
+ * @param {string} url
+ * @returns {string}
  */
-function enhanceMiddleware(middleware, _server) {
-  return (req, res, next) => {
-    const { url } = req;
-    if (url && url.startsWith("/assets/")) {
-      req.url = url.replaceAll("@@/", "../");
-    }
-    return middleware(req, res, next);
-  };
+function restoreAssetURL(url) {
+  return replaceString(url, "@@/", "../");
 }
 
 module.exports = assetPlugin;
-module.exports.enhanceMiddleware = enhanceMiddleware;
+module.exports.restoreAssetURL = restoreAssetURL;

packages/metro-config/src/assetPlugins/rewriteAssetURLs.js

@@ -9,7 +9,8 @@
  * fail to resolve them.
  *
  * In 0.67, they introduced a query parameter, `unstable_path`, specifically to
- * handle such cases.
+ * handle such cases. However, this isn't properly fleshed out and only works in
+ * server mode. Bundling with this plugin may cause issues with asset paths.
  *
  * For more details, see https://github.com/facebook/metro/issues/290.
  *

packages/metro-config/src/index.js

@@ -3,7 +3,6 @@
 
 const {
   findMetroPath,
-  getMetroVersion,
   requireModuleFromMetro,
 } = require("@rnx-kit/tools-react-native/metro");
 const fs = require("node:fs");
@@ -12,7 +11,10 @@ const { applyExpoWorkarounds, isExpoConfig } = require("./expoConfig");
 
 /**
  * @typedef {import("metro-config").MetroConfig} MetroConfig;
- * @typedef {MetroConfig & { platform?: string; }} InputConfig;
+ * @typedef {MetroConfig & {
+ *   platform?: string;
+ *   unstable_allowAssetsOutsideProjectRoot?: boolean;
+ * }} InputConfig;
  */
 
 /** Packages that must be resolved to one specific copy. */
@@ -281,18 +283,6 @@ function exclusionList(additionalExclusions = [], projectRoot = process.cwd()) {
   ];
 }
 
-/**
- * Returns whether Metro supports the `unstable_path` query parameter.
- * @param {string} projectRoot
- * @returns {boolean}
- */
-function supportsAssetPathQueryParam(projectRoot) {
-  // https://github.com/facebook/metro/commit/f3d1157bacc341dff82efea2f70b634141105fc0
-  const version = getMetroVersion(projectRoot) || "0.0.0";
-  const [major = 0, minor = 0] = version.split(".");
-  return Number(major) * 1000 + Number(minor) >= 67;
-}
-
 /**
  * @param {string} projectRoot
  * @param {InputConfig} inputConfig
@@ -322,13 +312,16 @@ function additionalConfig(projectRoot, inputConfig) {
     watchFolders: inputConfig.watchFolders ?? defaultWatchFolders(),
   };
 
-  if (supportsAssetPathQueryParam(projectRoot)) {
+  if (inputConfig.unstable_allowAssetsOutsideProjectRoot) {
+    // We currently cannot enable this by default because it affects the
+    // structure of the assets folder, breaking release builds.
     transformer.assetPlugins = [
       require.resolve("./assetPlugins/rewriteAssetURLs.js"),
     ];
   } else {
-    const { enhanceMiddleware } = require("./assetPlugins/escapeAssetURLs.js");
-    config.server = { enhanceMiddleware };
+    const { restoreAssetURL } = require("./assetPlugins/escapeAssetURLs.js");
+    config.server = { rewriteRequestUrl: restoreAssetURL };
+
     transformer.assetPlugins = [
       require.resolve("./assetPlugins/escapeAssetURLs.js"),
     ];

packages/metro-config/test/assetPlugins/escapeAssetURLs.test.ts

@@ -1,15 +1,12 @@
 import type { AssetData } from "metro";
-import type { Middleware } from "metro-config";
-import type Server from "metro/private/Server";
 import { equal } from "node:assert/strict";
-import type { IncomingMessage, ServerResponse } from "node:http";
 import { describe, it } from "node:test";
 import {
-  enhanceMiddleware,
   default as escapeAssetURL,
+  restoreAssetURL,
 } from "../../src/assetPlugins/escapeAssetURLs.js";
 
-describe("assetPluginForMonorepos/escapeRelativePaths", () => {
+describe("assetPlugins/escapeRelativePaths", () => {
   const cases = [
     ["/assets/./node_modules", "/assets/./node_modules"],
     ["/assets/../node_modules", "/assets/@@/node_modules"],
@@ -29,22 +26,7 @@ describe("assetPluginForMonorepos/escapeRelativePaths", () => {
 
   it("unescapes `..` in URLs", () => {
     for (const [output, input] of cases) {
-      const middleware: Middleware = (req: Middleware) => {
-        equal("url" in req && req.url, output);
-        return middleware;
-      };
-      const server = {
-        _config: { transformer: { assetPlugins: [] } },
-      } as unknown as Server;
-
-      const incoming = { url: input } as IncomingMessage;
-      const response = {} as ServerResponse;
-
-      enhanceMiddleware(middleware, server)(
-        incoming,
-        response,
-        () => undefined
-      );
+      equal(restoreAssetURL(input), output);
     }
   });
 });

packages/metro-config/test/assetPlugins/rewriteAssetURLs.test.ts

@@ -3,7 +3,7 @@ import { deepEqual } from "node:assert/strict";
 import { describe, it } from "node:test";
 import rewriteRequest from "../../src/assetPlugins/rewriteAssetURLs.js";
 
-describe("assetPluginForMonorepos/rewriteRequestWithQueryParam", () => {
+describe("assetPlugins/rewriteRequestWithQueryParam", () => {
   const cases = [
     ["/assets/./node_modules", "/assets/./node_modules"],
     ["/assets/../node_modules", "/assets?unstable_path=../node_modules"],

packages/metro-config/test/index.test.ts

@@ -420,7 +420,7 @@ describe("makeMetroConfig()", () => {
     deepEqual(config.resolver.blockList, blockList);
 
     deepEqual(config.transformer.assetPlugins, [
-      require.resolve("../src/assetPlugins/rewriteAssetURLs.js"),
+      require.resolve("../src/assetPlugins/escapeAssetURLs.js"),
     ]);
 
     const opts = { dev: false, hot: true, platform: undefined } as const;
@@ -494,7 +494,7 @@ describe("makeMetroConfig()", () => {
     deepEqual(config.resolver.blockList, blockList);
 
     deepEqual(config.transformer.assetPlugins, [
-      require.resolve("../src/assetPlugins/rewriteAssetURLs.js"),
+      require.resolve("../src/assetPlugins/escapeAssetURLs.js"),
     ]);
 
     const opts = { dev: false, hot: true, platform: undefined } as const;