diff --git a/packages/metro-file-map/API.md b/packages/metro-file-map/API.md index 87af82ff8c..fa29769a4b 100644 --- a/packages/metro-file-map/API.md +++ b/packages/metro-file-map/API.md @@ -233,7 +233,7 @@ export class HastePlugin implements HasteMap, FileMapPlugin getModuleNameByPath(mixedPath: Path): null | undefined | string; getPackage(name: string, platform: null | undefined | string, _supportsNativePlatform?: null | undefined | boolean): null | undefined | Path; getSerializableSnapshot(): null; - getWorker(): FileMapPluginWorker; + getWorker(): null | undefined | FileMapPluginWorker; initialize(opts: FileMapPluginInitOptions): Promise; readonly name: 'haste'; onChanged(delta: ReadonlyFileSystemChanges): void; diff --git a/packages/metro-file-map/src/__tests__/worker-test.js b/packages/metro-file-map/src/__tests__/worker-test.js index 8e57ac90da..1418aadb0a 100644 --- a/packages/metro-file-map/src/__tests__/worker-test.js +++ b/packages/metro-file-map/src/__tests__/worker-test.js @@ -17,6 +17,7 @@ import {HastePlugin} from '..'; import {Worker} from '../worker'; import * as path from 'node:path'; import * as vm from 'node:vm'; +import nullthrows from 'nullthrows'; jest.mock('node:fs', () => { const path = require('node:path'); @@ -98,10 +99,12 @@ function workerWithHaste( ) { return new Worker({ plugins: [ - new HastePlugin({ - ...defaultHasteConfig, - ...hasteOverrides, - }).getWorker().worker, + nullthrows( + new HastePlugin({ + ...defaultHasteConfig, + ...hasteOverrides, + }).getWorker(), + ).worker, ], }).processFile({ ...message, diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index a62ae0e7e4..926bd5b3b0 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -172,7 +172,7 @@ export type { // This should be bumped whenever a code change to `metro-file-map` itself // would cause a change to the cache data structure and/or content (for a given // filesystem state and build parameters). -const CACHE_BREAKER = '15'; +const CACHE_BREAKER = '16'; const CHANGE_INTERVAL = 30; diff --git a/packages/metro-file-map/src/lib/FileProcessor.js b/packages/metro-file-map/src/lib/FileProcessor.js index 72bc41346f..5cafd2a021 100644 --- a/packages/metro-file-map/src/lib/FileProcessor.js +++ b/packages/metro-file-map/src/lib/FileProcessor.js @@ -53,6 +53,8 @@ const NODE_MODULES_SEP = 'node_modules' + sep; const MAX_FILES_PER_WORKER = 100; export class FileProcessor { + // Whether any plugin's worker runs on a batch, rather than on demand. + #hasEagerPluginWorker: boolean; #maxFilesPerWorker: number; #maxWorkers: number; #perfLogger: ?PerfLogger; @@ -72,6 +74,9 @@ export class FileProcessor { this.#maxFilesPerWorker = opts.maxFilesPerWorker ?? MAX_FILES_PER_WORKER; this.#maxWorkers = opts.maxWorkers; this.#pluginWorkers = opts.pluginWorkers ?? []; + this.#hasEagerPluginWorker = this.#pluginWorkers.some( + plugin => plugin.lazy !== true, + ); this.#inBandWorker = new Worker({ plugins: this.#pluginWorkers.map(plugin => plugin.worker), }); @@ -90,6 +95,17 @@ export class FileProcessor { }> { const errors = []; + if (req.maybeReturnContent) { + throw new Error( + 'Batch processing does not support returning file contents', + ); + } + + if (!req.computeSha1 && !this.#hasEagerPluginWorker) { + // No file needs a worker. + return {errors}; + } + const workerJobs = files .map(([normalFilePath, fileMetadata]) => { const maybeWorkerInput = this.#getWorkerInput( @@ -110,12 +126,6 @@ export class FileProcessor { ); const batchWorker = this.#getBatchWorker(numWorkers); - if (req.maybeReturnContent) { - throw new Error( - 'Batch processing does not support returning file contents', - ); - } - await Promise.all( workerJobs.map(([workerInput, fileMetadata]) => { return batchWorker diff --git a/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js b/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js index 2dfbd426d3..000933e266 100644 --- a/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js +++ b/packages/metro-file-map/src/lib/__tests__/FileProcessor-test.js @@ -20,6 +20,7 @@ import H from '../../constants'; import DependencyPlugin from '../../plugins/DependencyPlugin'; import HastePlugin from '../../plugins/HastePlugin'; import path from 'node:path'; +import nullthrows from 'nullthrows'; const MockJestWorker = jest.fn().mockImplementation(() => ({ processFile: async () => ({}), @@ -276,6 +277,43 @@ describe('processBatch', () => { expect(mockWorkerFn).not.toHaveBeenCalled(); }); + test.each([ + ['no plugins', [] as ReadonlyArray], + [ + 'only lazy plugins', + [ + { + worker: {modulePath: 'mock-lazy-plugin', setupArgs: {}}, + filter: () => true, + lazy: true, + }, + ], + ], + ])( + 'reads no file without a SHA-1 to compute and with %s', + async (_, pluginWorkers) => { + const readKeys: Array = []; + const fileMetadata = new Proxy( + [123, 234, 0, null, 0, null], + { + get(target, key, receiver) { + readKeys.push(key); + return Reflect.get(target, key, receiver); + }, + }, + ); + const processor = new FileProcessor({...defaultOptions, pluginWorkers}); + + await processor.processBatch([[p('src/index.js'), fileMetadata]], { + computeSha1: false, + maybeReturnContent: false, + }); + + expect(readKeys).toEqual([]); + expect(mockWorkerFn).not.toHaveBeenCalled(); + }, + ); + describe('node_modules files', () => { const nodeModulesFiles = (): Array<[string, FileMetadata]> => [ [p('node_modules/pkg/index.js'), [123, 234, 0, null, 0, null]], @@ -335,13 +373,16 @@ describe('processBatch', () => { test('built-in plugins only run on them when a SHA-1 is needed', async () => { const pluginWorkers = [ - new HastePlugin({ - enableHastePackages: true, - failValidationOnConflicts: false, - hasteImplModulePath: require.resolve('../../__tests__/haste_impl.js'), - platforms: new Set(), - rootDir: defaultOptions.rootDir, - }).getWorker(), + nullthrows( + new HastePlugin({ + enableHastePackages: true, + failValidationOnConflicts: false, + hasteImplModulePath: + require.resolve('../../__tests__/haste_impl.js'), + platforms: new Set(), + rootDir: defaultOptions.rootDir, + }).getWorker(), + ), new DependencyPlugin({ computeDependencies: true, dependencyExtractor: null, diff --git a/packages/metro-file-map/src/plugins/HastePlugin.js b/packages/metro-file-map/src/plugins/HastePlugin.js index 64a7d93fdd..207587fc02 100644 --- a/packages/metro-file-map/src/plugins/HastePlugin.js +++ b/packages/metro-file-map/src/plugins/HastePlugin.js @@ -108,14 +108,16 @@ export default class HastePlugin await new Promise(setImmediate); } } - this.#getModuleNameByPath = mixedPath => { - const result = opts.getOrComputeFileData(mixedPath); - return result.exists && - result.type === 'f' && - typeof result.data === 'string' - ? result.data - : null; - }; + this.#getModuleNameByPath = this.#hasHasteNames() + ? mixedPath => { + const result = opts.getOrComputeFileData(mixedPath); + return result.exists && + result.type === 'f' && + typeof result.data === 'string' + ? result.data + : null; + } + : () => null; this.#perfLogger?.point('constructHasteMap_end'); this.#perfLogger?.annotate({int: {hasteFiles}}); } @@ -491,7 +493,17 @@ export default class HastePlugin ]); } - getWorker(): FileMapPluginWorker { + // Files have Haste names only through `hasteImplModulePath` or, for + // `package.json`, Haste packages. + #hasHasteNames(): boolean { + return this.#hasteImplModulePath != null || this.#enableHastePackages; + } + + getWorker(): ?FileMapPluginWorker { + if (!this.#hasHasteNames()) { + // Nothing to compute, so no file needs reading for this plugin. + return null; + } return { worker: { modulePath: require.resolve('./haste/worker.js'), diff --git a/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js b/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js index 00421797d7..ba68563ee5 100644 --- a/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js +++ b/packages/metro-file-map/src/plugins/haste/__tests__/HastePlugin-test.js @@ -50,6 +50,9 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { let HasteMap: Class; let DuplicateHasteCandidatesError; + const hasteImplModulePath = + require.resolve('../../../__tests__/haste_impl.js'); + const opts = { enableHastePackages: false, hasteImplModulePath: null, @@ -95,6 +98,29 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { expect(hasteMap.getModule('NameForFoo')).toEqual(p('/root/project/Foo.js')); }); + test.each([ + ['a hasteImplModulePath', {hasteImplModulePath}], + ['Haste packages', {enableHastePackages: true}], + ])('has a worker with %s', (_, overrides) => { + expect(new HasteMap({...opts, ...overrides}).getWorker()).not.toBeNull(); + }); + + test('has no worker or module names without a hasteImplModulePath or Haste packages', async () => { + const hasteMap = new HasteMap(opts); + expect(hasteMap.getWorker()).toBeNull(); + const getOrComputeFileData = jest.fn(); + await hasteMap.initialize({ + files: { + fileIterator: jest.fn().mockReturnValue([]), + lookup: jest.fn(), + }, + getOrComputeFileData, + pluginState: null, + }); + expect(hasteMap.getModuleNameByPath(p('/root/Foo.js'))).toBeNull(); + expect(getOrComputeFileData).not.toHaveBeenCalled(); + }); + describe('remove file', () => { let hasteMap: HasteMapType; let removeFile: (path: CanonicalPath, name: ?string) => void; @@ -184,7 +210,7 @@ describe.each([['win32'], ['posix']])('HastePlugin on %s', platform => { let getOrComputeFileData; beforeEach(async () => { - hasteMap = new HasteMap(opts); + hasteMap = new HasteMap({...opts, hasteImplModulePath}); getOrComputeFileData = jest.fn().mockReturnValue(null); await hasteMap.initialize({