Skip to content

Commit 10a5047

Browse files
committed
metro-file-map: Let plugins report what changed in a batch, on the change event
Stacked on #1952. ## Summary A file map plugin often knows things about a batch of changes that nothing else can work out. The motivating case is Haste - #1952 records the Haste names a resolution looks up, and to invalidate from those a consumer needs to know which names were rebound by a change. It can't derive that from the `ChangeEvent`, which carries paths and modified times, and by the time it sees the event a removed file's name is already gone from the map. `HastePlugin` has it to hand - its `onChanged` is given each removed file's old name and each added file's new one. This gives plugins a way to say so. `FileMapPlugin.onChanged` may now return a summary, and the file map publishes what each plugin returned as `ChangeEvent.pluginChanges`, by plugin name. A plugin that returns nothing is absent, so existing plugins are unaffected - the summary is a third type parameter on `FileMapPlugin` that defaults to `void`. `getPluginChanges(event, plugin)` reads an entry with the plugin's own summary type, so a consumer never writes the name or casts. The summary describes exactly the batch in `changes`. Plugins are already updated synchronously, immediately before the emit, so there's no ordering for a consumer to get wrong - which is the problem with the alternative of asking the plugin afterwards. There are two emit sites: the batched watch path updates plugins inline, and a recrawl updates them inside `#applyFileDelta`, which now returns what they reported along with the changes. The initial build goes through `#applyFileDelta` too but emits nothing, so its summaries go unused. `HastePlugin` is the first user. It reports `changedNames`, the names bound or unbound for any platform by the files added and removed - a bare name, matching what #1952 records. Plugin names now have to be unique across all of a file map's plugins. That was only checked for plugins with a worker, but persisted plugin state is keyed by name for every plugin, so two that shared a name would already have overwritten each other's state. Two things worth knowing: - On a cold start `onChanged` sees every file as added, so `HastePlugin` builds a set of every name for a summary nobody reads. That's nothing where Haste is inert, which is Metro's default, and transient otherwise. If it matters, the alternative is an optional method the file map calls only for a batch it's about to emit. - A modified file reaches plugins in `modifiedFiles` with its new data only, and `HastePlugin.onChanged` doesn't read that list. So an edit that changes a file's Haste name (a docblock, or a global package's `name`) is neither applied to the map nor reported here. That predates this, and can't happen with a path-based `hasteImpl`. `FileSystemChangeAggregator` keeps each file's pre-batch metadata, so giving plugins the old value is feasible, separately. Nothing consumes this yet. ## Test plan ``` yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler packages/metro/src/integration_tests packages/metro/src/__tests__ packages/metro/src/Server yarn flow check yarn typecheck-ts yarn verify-api-snapshots ``` - `HastePlugin-test.js`: `onChanged` reports the names bound and unbound, and nothing when no file with a name was added or removed. - `index-test.js`: in watch mode an added and a removed file put both names on the event, read back through `getPluginChanges`, and plugins with nothing to say are absent. A recrawl publishes what plugins reported too. Two plugins sharing a name throw, with or without a worker. Changelog: Internal
1 parent f4682e7 commit 10a5047

7 files changed

Lines changed: 229 additions & 42 deletions

File tree

‎packages/metro-file-map/API.md‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export type CacheManagerWriteOptions = Readonly<{
4747
export type ChangeEvent = Readonly<{
4848
logger?: null | undefined | RootPerfLogger;
4949
changes: ReadonlyFileSystemChanges<Readonly<ChangedFileMetadata>>;
50+
pluginChanges: ReadonlyMap<string, unknown>;
5051
rootDir: string;
5152
}>;
5253

@@ -191,6 +192,25 @@ interface FileSystem_2 {
191192
}
192193
export {FileSystem_2 as FileSystem};
193194

195+
export function getPluginChanges<ChangeSummary>(
196+
event: ChangeEvent,
197+
plugin: FileMapPlugin<
198+
/**
199+
* > | plugin: FileMapPlugin<empty, empty, ChangeSummary>,
200+
* | ^^^^^ Unsupported feature: Translating "empty type" is currently not supported.
201+
**/
202+
any,
203+
/**
204+
* > | plugin: FileMapPlugin<empty, empty, ChangeSummary>,
205+
* | ^^^^^ Unsupported feature: Translating "empty type" is currently not supported.
206+
**/
207+
any,
208+
ChangeSummary
209+
>,
210+
): ChangeSummary | void;
211+
212+
export type HasteChanges = Readonly<{changedNames: ReadonlySet<string>}>;
213+
194214
export class HasteConflictsError extends Error {
195215
constructor(conflicts: ReadonlyArray<HasteConflict>);
196216
getDetailedMessage(pathsRelativeToRoot: null | undefined | string): string;
@@ -209,7 +229,7 @@ export type HasteMapItem = {
209229
[platform: string]: HasteMapItemMetadata;
210230
};
211231

212-
export class HastePlugin implements HasteMap, FileMapPlugin<null, string | null> {
232+
export class HastePlugin implements HasteMap, FileMapPlugin<null, string | null, HasteChanges | void> {
213233
constructor(options: HasteMapOptions);
214234
assertValid(): void;
215235
computeConflicts(): Array<HasteConflict>;
@@ -221,7 +241,7 @@ export class HastePlugin implements HasteMap, FileMapPlugin<null, string | null>
221241
getWorker(): FileMapPluginWorker;
222242
initialize(opts: FileMapPluginInitOptions<null, string | null>): Promise<void>;
223243
readonly name: 'haste';
224-
onChanged(delta: ReadonlyFileSystemChanges<null | undefined | string>): void;
244+
onChanged(delta: ReadonlyFileSystemChanges<null | undefined | string>): HasteChanges | void;
225245
setModule(id: string, module: HasteMapItemMetadata): void;
226246
}
227247

@@ -247,15 +267,16 @@ export type HealthCheckResult =
247267

248268
export type InputFileMapPlugin = FileMapPlugin<
249269
/**
250-
* > | export type InputFileMapPlugin = FileMapPlugin<empty, empty>;
270+
* > | export type InputFileMapPlugin = FileMapPlugin<empty, empty, unknown>;
251271
* | ^^^^^ Unsupported feature: Translating "empty type" is currently not supported.
252272
**/
253273
any,
254274
/**
255-
* > | export type InputFileMapPlugin = FileMapPlugin<empty, empty>;
275+
* > | export type InputFileMapPlugin = FileMapPlugin<empty, empty, unknown>;
256276
* | ^^^^^ Unsupported feature: Translating "empty type" is currently not supported.
257277
**/
258-
any
278+
any,
279+
unknown
259280
>;
260281

261282
export type InputOptions = Readonly<{

‎packages/metro-file-map/src/__tests__/index-test.js‎

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import type {
2727
} from '../flow-types';
2828
import type {DependencyPlugin} from '../index';
2929
import type {default as FileMapT} from '../index';
30-
import type {HasteMapOptions} from '../plugins/HastePlugin';
30+
import type HastePluginT, {HasteMapOptions} from '../plugins/HastePlugin';
3131
import type {MockMapOptions} from '../plugins/MockPlugin';
3232
import typeof WorkerModule from '../worker';
3333

@@ -292,7 +292,7 @@ let buildNewFileMap: (
292292
) => Promise<{
293293
...BuildResult,
294294
fileMap: FileMapT,
295-
hasteMap: HasteMap,
295+
hasteMap: HastePluginT,
296296
mockMap: ?MockMap,
297297
dependencyPlugin: ?DependencyPlugin,
298298
}>;
@@ -445,6 +445,24 @@ describe('FileMap', () => {
445445
expect(FileMap.H).toBe(require('../constants').default);
446446
});
447447

448+
test('throws on plugins that share a name, with or without a worker', () => {
449+
const MockPlugin = require('../plugins/MockPlugin').default;
450+
const mockConfig = {
451+
console: globalThis.console,
452+
rootDir: defaultConfig.rootDir,
453+
mocksPattern: /__mocks__/,
454+
throwOnModuleCollision: false,
455+
};
456+
expect(
457+
() =>
458+
new FileMap({
459+
...defaultConfig,
460+
// MockPlugin has no worker, so holds no plugin data slot
461+
plugins: [new MockPlugin(mockConfig), new MockPlugin(mockConfig)],
462+
}),
463+
).toThrow('metro-file-map: Duplicate plugin name: mocks');
464+
});
465+
448466
test('ignores files given a pattern', async () => {
449467
mockFs[p('/project/fruits/Kiwi.js')] = `
450468
// Kiwi!
@@ -1683,6 +1701,28 @@ describe('FileMap', () => {
16831701
}
16841702
});
16851703

1704+
fm_it(
1705+
'publishes what each plugin reports about a batch on the change event',
1706+
async ({fileMap, hasteMap}) => {
1707+
const getPluginChanges = require('../lib/getPluginChanges').default;
1708+
const fruitsRoot = path.join('/', 'project', 'fruits');
1709+
mockFs[path.join(fruitsRoot, 'Tomato.js')] = '// Tomato!';
1710+
mockEmitters[fruitsRoot].emitFileEvent({
1711+
event: 'touch',
1712+
relativePath: 'Tomato.js',
1713+
metadata: MOCK_CHANGE_FILE,
1714+
});
1715+
mockDeleteFile(fruitsRoot, 'Banana.js');
1716+
const changeEvent = await waitForItToChange(fileMap);
1717+
1718+
expect(getPluginChanges(changeEvent, hasteMap)).toEqual({
1719+
changedNames: new Set(['Tomato', 'Banana']),
1720+
});
1721+
// Only plugins with something to say are present
1722+
expect([...changeEvent.pluginChanges.keys()]).toEqual([hasteMap.name]);
1723+
},
1724+
);
1725+
16861726
function mockDeleteFile(root: string, relativePath: string) {
16871727
const e = mockEmitters[root];
16881728
e.emitFileEvent({event: 'delete', relativePath});
@@ -1697,7 +1737,7 @@ describe('FileMap', () => {
16971737

16981738
function fm_it(
16991739
title: string,
1700-
fn: (fm: Readonly<{fileMap: FileMap, hasteMap: HasteMap}>) => unknown,
1740+
fn: (fm: Readonly<{fileMap: FileMap, hasteMap: HastePluginT}>) => unknown,
17011741
options?: FileMapTestOptions = {},
17021742
): void {
17031743
options = options || {};
@@ -1720,7 +1760,7 @@ describe('FileMap', () => {
17201760

17211761
fm_it.only = (
17221762
title: string,
1723-
fn: (fm: Readonly<{fileMap: FileMap, hasteMap: HasteMap}>) => unknown,
1763+
fn: (fm: Readonly<{fileMap: FileMap, hasteMap: HastePluginT}>) => unknown,
17241764
options?: FileMapTestOptions,
17251765
): void => fm_it(title, fn, {...options, only: true});
17261766

@@ -2510,7 +2550,13 @@ describe('FileMap', () => {
25102550
relativePath: 'tropical',
25112551
});
25122552

2513-
await waitForItToChange(fileMap);
2553+
const changeEvent = await waitForItToChange(fileMap);
2554+
2555+
// Plugins are updated inside the recrawl, and what they report
2556+
// still reaches the event
2557+
expect(
2558+
require('../lib/getPluginChanges').default(changeEvent, hasteMap),
2559+
).toEqual({changedNames: new Set(['Mango', 'Papaya'])});
25142560

25152561
// Verify crawl was called with the correct directory
25162562
expect(mockNodeCrawler).toHaveBeenNthCalledWith(

‎packages/metro-file-map/src/flow-types.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ export type ChangedFileMetadata = Readonly<{
111111
export type ChangeEvent = Readonly<{
112112
logger?: ?RootPerfLogger,
113113
changes: ReadonlyFileSystemChanges<Readonly<ChangedFileMetadata>>,
114+
/**
115+
* What each plugin reported about this batch of changes, by plugin name. A
116+
* plugin whose `onChanged` returned nothing is absent. Read an entry with
117+
* `getPluginChanges`, which knows its type.
118+
*/
119+
pluginChanges: ReadonlyMap<string, unknown>,
114120
rootDir: string,
115121
}>;
116122

@@ -278,19 +284,29 @@ export type V8Serializable = V8SerializablePrimitive | V8SerializableCollection;
278284
export interface FileMapPlugin<
279285
in SerializableState extends void | V8Serializable = void | V8Serializable,
280286
in PerFileData extends void | V8Serializable = void | V8Serializable,
287+
out ChangeSummary = void,
281288
> {
289+
// Must be unique among the plugins of a file map.
282290
readonly name: string;
283291
initialize(
284292
initOptions: FileMapPluginInitOptions<SerializableState, PerFileData>,
285293
): Promise<void>;
286294
assertValid(): void;
287-
onChanged(changes: ReadonlyFileSystemChanges<?PerFileData>): void;
295+
/**
296+
* Update the plugin's state for a batch of changes. It is called
297+
* synchronously, immediately before the batch is emitted as a `ChangeEvent`.
298+
*
299+
* A plugin may return a summary of what changed in terms only it knows,
300+
* such as the Haste names that were rebound, for the file map to publish as
301+
* `ChangeEvent.pluginChanges`. Return nothing if there is nothing to say.
302+
*/
303+
onChanged(changes: ReadonlyFileSystemChanges<?PerFileData>): ChangeSummary;
288304
getSerializableSnapshot(): void | V8Serializable;
289305
getCacheKey(): string;
290306
getWorker(): ?FileMapPluginWorker;
291307
}
292308

293-
export type InputFileMapPlugin = FileMapPlugin<empty, empty>;
309+
export type InputFileMapPlugin = FileMapPlugin<empty, empty, unknown>;
294310

295311
export interface MetadataWorker {
296312
processFile(

‎packages/metro-file-map/src/index.js‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ type InternalOptions = Readonly<{
123123

124124
type IndexedPlugin = Readonly<{
125125
// $FlowFixMe[unclear-type] Plugin types cannot be known statically
126-
plugin: FileMapPlugin<any, any>,
126+
plugin: FileMapPlugin<any, any, unknown>,
127127
dataIdx: ?number,
128128
}>;
129129
type InternalEnqueuedEvent = Readonly<
@@ -148,7 +148,9 @@ export type {FileDataPluginOptions} from './plugins/FileDataPlugin';
148148
export type {DependencyPluginOptions} from './plugins/DependencyPlugin';
149149
export {DuplicateHasteCandidatesError} from './plugins/haste/DuplicateHasteCandidatesError';
150150
export {HasteConflictsError} from './plugins/haste/HasteConflictsError';
151+
export {default as getPluginChanges} from './lib/getPluginChanges';
151152
export {default as HastePlugin} from './plugins/HastePlugin';
153+
export type {HasteChanges} from './plugins/HastePlugin';
152154

153155
export type {HasteMap} from './flow-types';
154156
export type {HealthCheckResult} from './Watcher';
@@ -321,19 +323,21 @@ export default class FileMap extends EventEmitter {
321323
const pluginWorkers: Array<FileMapPluginWorker> = [];
322324
const pluginDataIndices = new Map<string, number>();
323325
const plugins = options.plugins ?? [];
326+
const pluginNames = new Set<string>();
324327
for (const plugin of plugins) {
328+
// Plugin data slots, persisted plugin state and change summaries are all
329+
// addressed by name, so a second plugin of the same name would silently
330+
// take over the first's.
331+
invariant(
332+
!pluginNames.has(plugin.name),
333+
'metro-file-map: Duplicate plugin name: %s',
334+
plugin.name,
335+
);
336+
pluginNames.add(plugin.name);
325337
const maybeWorker = plugin.getWorker();
326338
const dataIdx = maybeWorker != null ? dataSlot++ : null;
327339
indexedPlugins.push({plugin, dataIdx});
328340
if (dataIdx != null) {
329-
// Crawlers address plugin data by name, so names must be unique among
330-
// plugins holding a slot - otherwise a crawler would silently write to
331-
// the shadowed plugin's slot.
332-
invariant(
333-
!pluginDataIndices.has(plugin.name),
334-
'metro-file-map: Duplicate plugin name: %s',
335-
plugin.name,
336-
);
337341
pluginDataIndices.set(plugin.name, dataIdx);
338342
}
339343
if (maybeWorker != null) {
@@ -515,7 +519,9 @@ export default class FileMap extends EventEmitter {
515519
]);
516520

517521
// Update `fileSystem` and plugins based on the file delta.
518-
const actualChanges = await this.#applyFileDelta(
522+
// Nothing is emitted for the initial build, so what plugins say about
523+
// it goes unused.
524+
const {changeAggregator: actualChanges} = await this.#applyFileDelta(
519525
fileSystem,
520526
plugins,
521527
fileDelta,
@@ -645,6 +651,27 @@ export default class FileMap extends EventEmitter {
645651
return null;
646652
}
647653

654+
/**
655+
* Bring every plugin up to date with a batch of changes, and collect what
656+
* each has to say about it for `ChangeEvent.pluginChanges`.
657+
*/
658+
#updatePlugins(
659+
changeAggregator: FileSystemChangeAggregator,
660+
): Map<string, unknown> {
661+
const pluginChanges = new Map<string, unknown>();
662+
this.#plugins.forEach(({plugin, dataIdx}) => {
663+
const summary = plugin.onChanged(
664+
changeAggregator.getMappedView(
665+
dataIdx != null ? metadata => metadata[dataIdx] : () => null,
666+
),
667+
);
668+
if (summary != null) {
669+
pluginChanges.set(plugin.name, summary);
670+
}
671+
});
672+
return pluginChanges;
673+
}
674+
648675
async #applyFileDelta(
649676
fileSystem: MutableFileSystem,
650677
plugins: ReadonlyArray<IndexedPlugin>,
@@ -653,7 +680,10 @@ export default class FileMap extends EventEmitter {
653680
removedFiles: ReadonlySet<CanonicalPath>,
654681
clocks?: WatchmanClocks,
655682
}>,
656-
): Promise<FileSystemChangeAggregator> {
683+
): Promise<{
684+
changeAggregator: FileSystemChangeAggregator,
685+
pluginChanges: ReadonlyMap<string, unknown>,
686+
}> {
657687
this.#startupPerfLogger?.point('applyFileDelta_start');
658688
const {changedFiles, removedFiles} = delta;
659689
this.#startupPerfLogger?.point('applyFileDelta_preprocess_start');
@@ -742,17 +772,11 @@ export default class FileMap extends EventEmitter {
742772
this.#startupPerfLogger?.point('applyFileDelta_add_end');
743773

744774
this.#startupPerfLogger?.point('applyFileDelta_updatePlugins_start');
745-
this.#plugins.forEach(({plugin, dataIdx}) => {
746-
plugin.onChanged(
747-
changeAggregator.getMappedView(
748-
dataIdx != null ? metadata => metadata[dataIdx] : () => null,
749-
),
750-
);
751-
});
775+
const pluginChanges = this.#updatePlugins(changeAggregator);
752776
this.#startupPerfLogger?.point('applyFileDelta_updatePlugins_end');
753777
this.#startupPerfLogger?.point('applyFileDelta_end');
754778

755-
return changeAggregator;
779+
return {changeAggregator, pluginChanges};
756780
}
757781

758782
/**
@@ -860,13 +884,7 @@ export default class FileMap extends EventEmitter {
860884
}
861885

862886
const _netChange = changeAggregator.getView();
863-
this.#plugins.forEach(({plugin, dataIdx}) => {
864-
plugin.onChanged(
865-
changeAggregator.getMappedView(
866-
dataIdx != null ? metadata => metadata[dataIdx] : () => null,
867-
),
868-
);
869-
});
887+
const pluginChanges = this.#updatePlugins(changeAggregator);
870888

871889
const toPublicMetadata = (
872890
metadata: Readonly<FileMetadata>,
@@ -893,6 +911,7 @@ export default class FileMap extends EventEmitter {
893911
const changeEvent: ChangeEvent = {
894912
changes: changesWithMetadata,
895913
logger: hmrPerfLogger,
914+
pluginChanges,
896915
rootDir: this.#options.rootDir,
897916
};
898917
this.emit('change', changeEvent);
@@ -1039,7 +1058,10 @@ export default class FileMap extends EventEmitter {
10391058
}
10401059

10411060
// Reuse the same batch processing logic as build()
1042-
const recrawlChangeAggregator = await this.#applyFileDelta(
1061+
const {
1062+
changeAggregator: recrawlChangeAggregator,
1063+
pluginChanges: recrawlPluginChanges,
1064+
} = await this.#applyFileDelta(
10431065
fileSystem,
10441066
this.#plugins,
10451067
crawlResult,
@@ -1067,6 +1089,7 @@ export default class FileMap extends EventEmitter {
10671089
const changeEvent: ChangeEvent = {
10681090
changes: changesWithMetadata,
10691091
logger: null,
1092+
pluginChanges: recrawlPluginChanges,
10701093
rootDir: this.#options.rootDir,
10711094
};
10721095
this.emit('change', changeEvent);

0 commit comments

Comments
 (0)