Agree with custom transformers on the URL paths of assets

packages/metro-transform-worker/API.md

@@ -18,6 +18,7 @@ export type JsOutput = Readonly<{
     lineCount: number;
     map: VlqMap;
     functionMap: null | undefined | FBSourceFunctionMap;
+    unstable_assetUrlPath?: string | undefined;
   }>;
   type: JSFileType;
 }>;

packages/metro-transform-worker/src/__tests__/index-test.js

@@ -162,6 +162,30 @@ test('uses the indexed watch folder path for asset URLs', async () => {
   expect(result.output[0].data.code).toContain(
     '"httpServerLocation": "/assets/[metro-watchFolders]/1"',
   );
+  expect(result.output[0].data.unstable_assetUrlPath).toBe(
+    '[metro-watchFolders]/1/test.mp4',
+  );
+});
+
+test('reports the project-relative URL path of an asset given no URL path', async () => {
+  fs.mkdirSync(p('/root/external'), {recursive: true});
+  fs.writeFileSync(p('/root/external/test.mp4'), 'asset data');
+
+  const result = await Transformer.transform(
+    baseConfig,
+    p('/root'),
+    'external/test.mp4',
+    Buffer.from('asset data'),
+    {
+      ...baseTransformOptions,
+      type: 'asset',
+    },
+  );
+
+  expect(result.output[0].data.code).toContain(
+    '"httpServerLocation": "/assets/external"',
+  );
+  expect(result.output[0].data.unstable_assetUrlPath).toBe('external/test.mp4');
 });
 
 test('transforms a module with dependencies', async () => {

packages/metro-transform-worker/src/index.js

@@ -180,6 +180,9 @@ export type JsOutput = Readonly<{
     lineCount: number,
     map: VlqMap,
     functionMap: ?FBSourceFunctionMap,
+    // Assets only: the URL path the code was generated with. A temporary
+    // migration helper, to be removed.
+    unstable_assetUrlPath?: string,
   }>,
   type: JSFileType,
 }>;
@@ -539,12 +542,13 @@ async function transformAsset(
   context: TransformationContext,
 ): Promise<TransformResponse> {
   const {assetRegistryPath, assetPlugins} = context.config;
+  const urlPath = context.assetUrlPath ?? file.filename;
 
   const result = await assetTransformer.transform(
     getBabelTransformArgs(file, context),
     assetRegistryPath,
     assetPlugins,
-    context.assetUrlPath ?? undefined,
+    urlPath,
   );
 
   const jsFile = {
@@ -554,7 +558,14 @@ async function transformAsset(
     type: 'js/module/asset' as const,
   };
 
-  return transformJS(jsFile, context);
+  const response = await transformJS(jsFile, context);
+  return {
+    ...response,
+    output: response.output.map(output => ({
+      ...output,
+      data: {...output.data, unstable_assetUrlPath: urlPath},
+    })),
+  };
 }
 
 /**

packages/metro/src/DeltaBundler/Serializers/__tests__/getAssets-test.js

@@ -10,17 +10,15 @@
 
 jest.mock('../../../Assets');
 
-import {getAssetData, getAssetUrlPath} from '../../../Assets';
+import {getAssetData} from '../../../Assets';
 import getAssets from '../getAssets';
+import path from 'node:path';
 
 beforeEach(() => {
   getAssetData.mockImplementation(async (path, localPath) => ({
     path,
     localPath,
   }));
-  getAssetUrlPath.mockImplementation(
-    jest.requireActual('../../../Assets').getAssetUrlPath,
-  );
 });
 
 test('should return the bundle assets', async () => {
@@ -85,42 +83,43 @@ test('should return the bundle assets', async () => {
         ],
       },
     ],
-    [
-      '/external/6.png',
-      {
-        path: '/external/6.png',
-        output: [
-          {
-            type: 'js/module/asset',
-            data: {code: '//', lineCount: 1, map: [], functionMap: null},
-          },
-        ],
-      },
-    ],
   ]);
 
   expect(
     await getAssets(dependencies, {
       projectRoot: '/tmp',
-      watchFolders: ['/tmp', '/external'],
       processModuleFilter: () => true,
     }),
   ).toEqual([
     {path: '/tmp/3.png', localPath: '3.png'},
     {path: '/tmp/5.mov', localPath: '5.mov'},
-    {
-      path: '/external/6.png',
-      localPath: '[metro-watchFolders]/1/6.png',
-    },
   ]);
 });
 
-test('uses the id of a dynamic root for an asset within it', async () => {
+test('uses the URL path the transformer generated an asset with', async () => {
   const dependencies = new Map([
     [
-      '/other/imgs/7.png',
+      '/external/6.png',
+      {
+        path: '/external/6.png',
+        output: [
+          {
+            type: 'js/module/asset',
+            data: {
+              code: '//',
+              lineCount: 1,
+              map: [],
+              functionMap: null,
+              unstable_assetUrlPath: '[metro-watchFolders]/1/6.png',
+            },
+          },
+        ],
+      },
+    ],
+    [
+      '/external/7.png',
       {
-        path: '/other/imgs/7.png',
+        path: '/external/7.png',
         output: [
           {
             type: 'js/module/asset',
@@ -134,14 +133,10 @@ test('uses the id of a dynamic root for an asset within it', async () => {
   expect(
     await getAssets(dependencies, {
       projectRoot: '/tmp',
-      watchFolders: ['/tmp', '/external'],
-      dynamicRoots: [{id: '01234567', rootDir: '/other'}],
       processModuleFilter: () => true,
     }),
   ).toEqual([
-    {
-      path: '/other/imgs/7.png',
-      localPath: '[metro-watchFolders]/01234567/imgs/7.png',
-    },
+    {path: '/external/6.png', localPath: '[metro-watchFolders]/1/6.png'},
+    {path: '/external/7.png', localPath: path.join('..', 'external', '7.png')},
   ]);
 });

packages/metro/src/DeltaBundler/Serializers/getAssets.js

@@ -10,10 +10,9 @@
  */
 
 import type {AssetData} from '../../Assets';
-import type {DynamicRoot} from '../../lib/dynamicRoots';
 import type {Module, ReadOnlyDependencies} from '../types';
 
-import {getAssetData, getAssetUrlPath} from '../../Assets';
+import {getAssetData} from '../../Assets';
 import {getJsOutput, isJsModule} from './helpers/js';
 import path from 'node:path';
 
@@ -23,8 +22,6 @@ type Options = {
   platform: ?string,
   projectRoot: string,
   publicPath: string,
-  watchFolders: ReadonlyArray<string>,
-  dynamicRoots?: ReadonlyArray<DynamicRoot>,
 };
 
 export default async function getAssets(
@@ -35,21 +32,20 @@ export default async function getAssets(
   const {processModuleFilter} = options;
 
   for (const module of dependencies.values()) {
+    if (!isJsModule(module) || !processModuleFilter(module)) {
+      continue;
+    }
+    const output = getJsOutput(module);
+    const projectRelativePath = path.relative(options.projectRoot, module.path);
     if (
-      isJsModule(module) &&
-      processModuleFilter(module) &&
-      getJsOutput(module).type === 'js/module/asset' &&
-      path.relative(options.projectRoot, module.path) !== 'package.json'
+      output.type === 'js/module/asset' &&
+      projectRelativePath !== 'package.json'
     ) {
       promises.push(
         getAssetData(
           module.path,
-          getAssetUrlPath(
-            module.path,
-            options.projectRoot,
-            options.watchFolders,
-            options.dynamicRoots,
-          ),
+          // Transformers other than Metro's use the project-relative path.
+          output.data.unstable_assetUrlPath ?? projectRelativePath,
           options.assetPlugins,
           options.platform,
           options.publicPath,

packages/metro/src/DeltaBundler/Serializers/helpers/getSourceMapInfo.js

@@ -34,8 +34,12 @@ export default function getSourceMapInfo(
   readonly lineCount: number,
   readonly isIgnored: boolean,
 } {
+  const {code, functionMap, lineCount, map} = getJsOutput(module).data;
   return {
-    ...getJsOutput(module).data,
+    code,
+    functionMap,
+    lineCount,
+    map,
     isIgnored: options.shouldAddToIgnoreList(module),
     path: options?.getSourceUrl?.(module) ?? module.path,
     source: options.excludeSource ? '' : getModuleSource(module),

packages/metro/src/Server.js

@@ -444,8 +444,6 @@ export default class Server {
       platform,
       projectRoot: this._config.projectRoot,
       publicPath: this._config.transformer.publicPath,
-      watchFolders: this._config.watchFolders,
-      dynamicRoots: this._bundler.getBundler().getDynamicRoots(),
     });
   }
 

packages/metro/src/Server/__tests__/Server-test.js

@@ -1640,7 +1640,6 @@ describe('processRequest', () => {
         expect.anything(),
         expect.objectContaining({
           projectRoot: p('/root'),
-          watchFolders: [p('/root')],
         }),
       );
     });

packages/metro/src/integration_tests/__tests__/build-test.js

@@ -35,6 +35,42 @@ test('builds a simple bundle', async () => {
   expect(result.assets).toBeUndefined();
 });
 
+test.each([
+  ['Metro transformer', null, /^\/assets\/\[metro-watchFolders\]\/\d+$/],
+  // Doesn't forward the URL path, so uses the project-relative one.
+  [
+    'custom transformer wrapping Metro transformer',
+    'wrapper-transformer.js',
+    /^\/assets\/\.\.$/,
+  ],
+])(
+  'agrees with getAssets on the URL of an asset outside projectRoot (%s)',
+  async (_name, transformer, expectedLocation) => {
+    const projectRoot = path.join(INPUT_PATH, 'external-asset');
+    const baseConfig = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+    const config = MetroConfig.mergeConfig(baseConfig, {
+      projectRoot,
+      ...(transformer != null
+        ? {transformerPath: path.join(projectRoot, transformer)}
+        : null),
+    });
+
+    const result = await Metro.runBuild(config, {
+      entry: 'index.js',
+      assets: true,
+    });
+
+    // $FlowFixMe[incompatible-use] - the bundle evaluates to the asset data
+    const {httpServerLocation} = execBundle(result.code);
+    expect(httpServerLocation).toMatch(expectedLocation);
+    expect(result.assets?.map(asset => asset.httpServerLocation)).toEqual([
+      httpServerLocation,
+    ]);
+  },
+);
+
 test('build a simple bundle with polyfills', async () => {
   const baseConfig = await Metro.loadConfig({
     config: require.resolve('../metro.config.js'),

packages/metro/src/integration_tests/basic_bundle/external-asset/index.js

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ * @flow strict
+ */
+
+'use strict';
+
+// Outside projectRoot, which is this directory.
+module.exports = require('../test.png');

packages/metro/src/integration_tests/basic_bundle/external-asset/wrapper-transformer.js

@@ -0,0 +1,31 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ * @flow strict-local
+ */
+
+'use strict';
+
+import type {
+  JsTransformerConfig,
+  JsTransformOptions,
+} from 'metro-transform-worker';
+
+// Wraps Metro's transformer, forwarding only the arguments it declares.
+const worker = require('metro-transform-worker');
+
+module.exports = {
+  getCacheKey: worker.getCacheKey,
+  transform: (
+    config: JsTransformerConfig,
+    projectRoot: string,
+    filename: string,
+    data: Buffer,
+    options: JsTransformOptions,
+  ): ReturnType<typeof worker.transform> =>
+    worker.transform(config, projectRoot, filename, data, options),
+};