From 0e51ad92f3f2fba3f90410d246009e5ade48db7c Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 22 Dec 2023 08:01:37 -0800 Subject: [PATCH] Restructure Graph - separate traversal from state mutation Summary: This is a significant restructuring of the internal operation of `Graph`. Currently, we asynchronously traverse, and resolve, from a set of entry/modified paths, updating the graph's state as we go. This change performs the same work in two distinct phases: 1. Draft: Asynchronously traverse the necessary modules, collecting transform and resolution results with *no* mutation - using a separate, ephemeral `GraphTraversal` instance to make that explicit. 2. Commit: Mutate graph state in a second, synchronous, depth-first traversal of the modified modules. The motivation for this is twofold: - Resolve a category of known bugs where a userland issue, such as a syntax error or unresolvable dependency, would interrupt traversal and potentially leave internal state *partially* updated *without sending an update to the client*. This will now fail in the draft phase, allowing `DeltaCalculator` to repeat the attempt when the userland issue is fixed. - Makes the operation of `Graph` easier to reason about, and opens up several opportunities for simplification (to follow). For example, the commit phase being synchronous means that ordering is deterministic, and that the need to track `earlyInverseDependencies` is obviated. Some of the issues fixed here were introduced by Graph delta pruning in D45691844, but others are longstanding. New unit tests cover the major cases. Changelog: ``` **[Fix]**: Fast Refresh may fail or lose modifications after correcting transform or resolution issues in user code. ``` Reviewed By: motiz88 Differential Revision: D51665495 fbshipit-source-id: 0bd45c6e56d148fcc64bca60cd86ed6034b8034d --- packages/metro/src/DeltaBundler/Graph.js | 314 ++++++++---------- .../src/DeltaBundler/__tests__/Graph-test.js | 241 ++++++++++++++ .../__tests__/buildSubgraph-test.js | 177 ++++++++++ .../metro/src/DeltaBundler/buildSubgraph.js | 154 +++++++++ packages/metro/src/DeltaBundler/types.flow.js | 19 +- 5 files changed, 729 insertions(+), 176 deletions(-) create mode 100644 packages/metro/src/DeltaBundler/__tests__/buildSubgraph-test.js create mode 100644 packages/metro/src/DeltaBundler/buildSubgraph.js diff --git a/packages/metro/src/DeltaBundler/Graph.js b/packages/metro/src/DeltaBundler/Graph.js index 73d2010c38..bd13bf7e27 100644 --- a/packages/metro/src/DeltaBundler/Graph.js +++ b/packages/metro/src/DeltaBundler/Graph.js @@ -37,17 +37,14 @@ import type { GraphInputOptions, MixedOutput, Module, + ModuleData, Options, TransformInputOptions, - TransformResultDependency, } from './types.flow'; -import { - deriveAbsolutePathFromContext, - fileMatchesContext, -} from '../lib/contextModule'; +import {fileMatchesContext} from '../lib/contextModule'; import CountingSet from '../lib/CountingSet'; -import * as path from 'path'; +import {buildSubgraph} from './buildSubgraph'; const invariant = require('invariant'); const nullthrows = require('nullthrows'); @@ -80,7 +77,7 @@ export type Result = { * files have been modified. This allows to return the added modules before the * modified ones (which is useful for things like Hot Module Reloading). **/ -type Delta = $ReadOnly<{ +type Delta = $ReadOnly<{ // `added` and `deleted` are mutually exclusive. // Internally, a module can be in both `modified` and (either) `added` or // `deleted`. We fix this up before returning the delta to the client. @@ -91,6 +88,8 @@ type Delta = $ReadOnly<{ // A place to temporarily track inverse dependencies for a module while it is // being processed but has not been added to `graph.dependencies` yet. earlyInverseDependencies: Map>, + + moduleData: Map>, }>; type InternalOptions = $ReadOnly<{ @@ -122,6 +121,14 @@ function getInternalOptions({ }; } +function isWeakOrLazy( + dependency: Dependency, + options: InternalOptions, +): boolean { + const asyncType = dependency.data.data.asyncType; + return asyncType === 'weak' || (asyncType != null && options.lazy); +} + export class Graph { +entryPoints: $ReadOnlySet; +transformOptions: TransformInputOptions; @@ -165,13 +172,6 @@ export class Graph { paths: $ReadOnlyArray, options: Options, ): Promise> { - const delta = { - added: new Set(), - modified: new Set(), - deleted: new Set(), - earlyInverseDependencies: new Map>(), - }; - const internalOptions = getInternalOptions(options); // Record the paths that are part of the dependency graph before we start @@ -186,19 +186,48 @@ export class Graph { } } - for (const [path] of originalModules) { - // Traverse over modules that are part of the dependency graph. - // - // Note: A given path may not be part of the graph *at this time*, in - // particular it may have been removed since we started traversing, but - // in that case the path will be visited if and when we add it back to - // the graph in a subsequent iteration. - if (this.dependencies.has(path)) { - await this._traverseDependenciesForSingleFile( - path, - delta, - internalOptions, - ); + const allModifiedPaths = new Set(paths); + + const modifiedPathsInBaseGraph = paths.filter(path => + this.dependencies.has(path), + ); + + const moduleData = await buildSubgraph( + new Set(modifiedPathsInBaseGraph), + this.#resolvedContexts, + { + resolve: options.resolve, + transform: async (absolutePath, requireContext) => { + internalOptions.onDependencyAdd(); + const result = await options.transform(absolutePath, requireContext); + internalOptions.onDependencyAdded(); + return result; + }, + shouldTraverse: (dependency: Dependency) => { + if ( + // Don't traverse into any path that hasn't been modified and as is + // already in the graph. + this.dependencies.has(dependency.absolutePath) && + !allModifiedPaths.has(dependency.absolutePath) + ) { + return false; + } + return !options.shallow && !isWeakOrLazy(dependency, internalOptions); + }, + }, + ); + + const delta = { + added: new Set(), + modified: new Set(), + deleted: new Set(), + earlyInverseDependencies: new Map>(), + moduleData, + }; + + for (const modified of modifiedPathsInBaseGraph) { + if (this.dependencies.has(modified)) { + this._recursivelyCommitModule(modified, delta, internalOptions); } } @@ -232,8 +261,9 @@ export class Graph { // Module.dependencies is mutable, so it's not obviously the case // that referential equality implies no modification. However, we // only mutate dependencies in two cases: - // 1. Within _processModule. In that case, we always mutate a new - // module and set a new reference in this.dependencies. + // 1. Within _recursivelyCommitModule. In that case, we always + // mutate a new module and set a new reference in + // this.dependencies. // 2. During _releaseModule, when recursively removing // dependencies. In that case, we immediately discard the module // object. @@ -257,13 +287,6 @@ export class Graph { } async initialTraverseDependencies(options: Options): Promise> { - const delta = { - added: new Set(), - modified: new Set(), - deleted: new Set(), - earlyInverseDependencies: new Map>(), - }; - const internalOptions = getInternalOptions(options); invariant( @@ -280,10 +303,28 @@ export class Graph { this.#gc.color.set(path, 'black'); } - await Promise.all( - [...this.entryPoints].map((path: string) => - this._traverseDependenciesForSingleFile(path, delta, internalOptions), - ), + const moduleData = await buildSubgraph(this.entryPoints, new Map(), { + resolve: options.resolve, + transform: async (absolutePath, requireContext) => { + internalOptions.onDependencyAdd(); + const result = await options.transform(absolutePath, requireContext); + internalOptions.onDependencyAdded(); + return result; + }, + shouldTraverse: (dependency: Dependency) => + !options.shallow && !isWeakOrLazy(dependency, internalOptions), + }); + + const delta = { + added: new Set(), + modified: new Set(), + deleted: new Set(), + earlyInverseDependencies: new Map>(), + moduleData, + }; + + [...this.entryPoints].forEach((path: string) => + this._recursivelyCommitModule(path, delta, internalOptions), ); this.reorderGraph({ @@ -297,37 +338,19 @@ export class Graph { }; } - async _traverseDependenciesForSingleFile( - path: string, - delta: Delta, - options: InternalOptions, - ): Promise { - options.onDependencyAdd(); - - await this._processModule(path, delta, options); - - options.onDependencyAdded(); - } - - async _processModule( + _recursivelyCommitModule( path: string, - delta: Delta, + delta: Delta, options: InternalOptions, - ): Promise> { - const resolvedContext = this.#resolvedContexts.get(path); - - // Transform the file via the given option. - // TODO: Unbind the transform method from options - const result = await options.transform(path, resolvedContext); - - // Get the absolute path of all sub-dependencies (some of them could have been - // moved but maintain the same relative path). - const {dependencies: currentDependencies, resolvedContexts} = - this._resolveDependencies(path, result.dependencies, options); - + ): Module { + const currentModule = nullthrows(delta.moduleData.get(path)); const previousModule = this.dependencies.get(path); - const previousDependencies = previousModule?.dependencies ?? new Map(); + const { + dependencies: currentDependencies, + resolvedContexts, + ...transformResult + } = currentModule; const nextModule = { ...(previousModule ?? { @@ -335,10 +358,8 @@ export class Graph { delta.earlyInverseDependencies.get(path) ?? new CountingSet(), path, }), + ...transformResult, dependencies: new Map(previousDependencies), - getSource: result.getSource, - output: result.output, - unstable_transformResultKey: result.unstable_transformResultKey, }; // Update the module information. @@ -358,22 +379,21 @@ export class Graph { } // Diff dependencies (2/2): add dependencies that have changed or been added. - const addDependencyPromises = []; + let dependenciesAdded = false; for (const [key, curDependency] of currentDependencies) { const prevDependency = previousDependencies.get(key); if ( !prevDependency || !dependenciesEqual(prevDependency, curDependency, options) ) { - addDependencyPromises.push( - this._addDependency( - nextModule, - key, - curDependency, - resolvedContexts.get(key), - delta, - options, - ), + dependenciesAdded = true; + this._addDependency( + nextModule, + key, + curDependency, + resolvedContexts.get(key), + delta, + options, ); } } @@ -382,7 +402,7 @@ export class Graph { previousModule && !transfromOutputMayDiffer(previousModule, nextModule) && !dependenciesRemoved && - addDependencyPromises.length === 0 + !dependenciesAdded ) { // We have not operated on nextModule, so restore previousModule // to aid diffing. @@ -392,8 +412,6 @@ export class Graph { delta.modified.add(path); - await Promise.all(addDependencyPromises); - // Replace dependencies with the correctly-ordered version. As long as all // the above promises have resolved, this will be the same map but without // the added nondeterminism of promise resolution order. Because this @@ -406,19 +424,19 @@ export class Graph { 'Failed to add the correct dependencies', ); - nextModule.dependencies = currentDependencies; + nextModule.dependencies = new Map(currentDependencies); return nextModule; } - async _addDependency( + _addDependency( parentModule: Module, key: string, dependency: Dependency, requireContext: ?RequireContext, - delta: Delta, + delta: Delta, options: InternalOptions, - ): Promise { + ): void { const path = dependency.absolutePath; if (requireContext) { @@ -461,9 +479,7 @@ export class Graph { } delta.earlyInverseDependencies.set(path, new CountingSet()); - options.onDependencyAdd(); - module = await this._processModule(path, delta, options); - options.onDependencyAdded(); + module = this._recursivelyCommitModule(path, delta, options); this.dependencies.set(module.path, module); } @@ -486,7 +502,7 @@ export class Graph { parentModule: Module, key: string, dependency: Dependency, - delta: Delta, + delta: Delta, options: InternalOptions, ): void { parentModule.dependencies.delete(key); @@ -551,81 +567,6 @@ export class Graph { yield* this.#importBundleNodes.get(filePath)?.inverseDependencies ?? []; } - _resolveDependencies( - parentPath: string, - dependencies: $ReadOnlyArray, - options: InternalOptions, - ): { - dependencies: Map, - resolvedContexts: Map, - } { - const maybeResolvedDeps = new Map(); - const resolvedContexts = new Map(); - for (const dep of dependencies) { - let resolvedDep; - const key = dep.data.key; - - // `require.context` - const {contextParams} = dep.data; - if (contextParams) { - // Ensure the filepath has uniqueness applied to ensure multiple `require.context` - // statements can be used to target the same file with different properties. - const from = path.join(parentPath, '..', dep.name); - const absolutePath = deriveAbsolutePathFromContext(from, contextParams); - - const resolvedContext: RequireContext = { - from, - mode: contextParams.mode, - recursive: contextParams.recursive, - filter: new RegExp( - contextParams.filter.pattern, - contextParams.filter.flags, - ), - }; - - resolvedContexts.set(key, resolvedContext); - - resolvedDep = { - absolutePath, - data: dep, - }; - } else { - try { - resolvedDep = { - absolutePath: options.resolve(parentPath, dep).filePath, - data: dep, - }; - } catch (error) { - // Ignore unavailable optional dependencies. They are guarded - // with a try-catch block and will be handled during runtime. - if (dep.data.isOptional !== true) { - throw error; - } - } - } - - if (maybeResolvedDeps.has(key)) { - throw new Error( - `resolveDependencies: Found duplicate dependency key '${key}' in ${parentPath}`, - ); - } - maybeResolvedDeps.set(key, resolvedDep); - } - - const resolvedDeps = new Map(); - // Return just the dependencies we successfully resolved. - // FIXME: This has a bad bug affecting all dependencies *after* an unresolved - // optional dependency. We'll need to propagate the nulls all the way to the - // serializer and the require() runtime to keep the dependency map from being - // desynced from the contents of the module. - for (const [key, resolvedDep] of maybeResolvedDeps) { - if (resolvedDep) { - resolvedDeps.set(key, resolvedDep); - } - } - return {dependencies: resolvedDeps, resolvedContexts}; - } - /** * Re-traverse the dependency graph in DFS order to reorder the modules and * guarantee the same order between runs. This method mutates the passed graph. @@ -726,8 +667,7 @@ export class Graph { options: InternalOptions, ): Iterator> { for (const dependency of module.dependencies.values()) { - const asyncType = dependency.data.data.asyncType; - if (asyncType === 'weak' || (options.lazy && asyncType != null)) { + if (isWeakOrLazy(dependency, options)) { continue; } yield nullthrows(this.dependencies.get(dependency.absolutePath)); @@ -737,7 +677,37 @@ export class Graph { // Delete an unreachable module (and its outbound edges) from the graph // immediately. // Called when the reference count of a module has reached 0. - _releaseModule(module: Module, delta: Delta, options: InternalOptions) { + _releaseModule( + module: Module, + delta: Delta, + options: InternalOptions, + ) { + if (!delta.moduleData.has(module.path)) { + // Before releasing a module, take a snapshot of the data we might need + // to reintroduce it to the graph later in this commit. As it is not + // already present in moduleData we can infer it has not been modified, + // so the transform output and dependencies we copy here are current. + const {dependencies, getSource, output, unstable_transformResultKey} = + module; + + const resolvedContexts: Map = new Map(); + for (const [key, dependency] of dependencies) { + const resolvedContext = this.#resolvedContexts.get( + dependency.absolutePath, + ); + if (resolvedContext != null) { + resolvedContexts.set(key, resolvedContext); + } + } + delta.moduleData.set(module.path, { + dependencies: new Map(dependencies), + resolvedContexts, + getSource, + output, + unstable_transformResultKey, + }); + } + for (const [key, dependency] of module.dependencies) { this._removeDependency(module, key, dependency, delta, options); } @@ -746,7 +716,7 @@ export class Graph { } // Delete an unreachable module from the graph. - _freeModule(module: Module, delta: Delta) { + _freeModule(module: Module, delta: Delta) { if (delta.added.has(module.path)) { // Mark the deletion by clearing a prior addition. delta.added.delete(module.path); @@ -774,7 +744,7 @@ export class Graph { } // Collect any unreachable cycles in the graph. - _collectCycles(delta: Delta, options: InternalOptions) { + _collectCycles(delta: Delta, options: InternalOptions) { // Mark recursively from roots (trial deletion) for (const path of this.#gc.possibleCycleRoots) { const module = nullthrows(this.dependencies.get(path)); @@ -846,7 +816,7 @@ export class Graph { } } - _collectWhite(module: Module, delta: Delta) { + _collectWhite(module: Module, delta: Delta) { const color = nullthrows(this.#gc.color.get(module.path)); if (color === 'white' && !this.#gc.possibleCycleRoots.has(module.path)) { this.#gc.color.set(module.path, 'black'); diff --git a/packages/metro/src/DeltaBundler/__tests__/Graph-test.js b/packages/metro/src/DeltaBundler/__tests__/Graph-test.js index 68d50f47da..b904f768e0 100644 --- a/packages/metro/src/DeltaBundler/__tests__/Graph-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/Graph-test.js @@ -41,6 +41,7 @@ import type { Options, ReadOnlyDependencies, ReadOnlyGraph, + TransformFn, TransformResultDependency, TransformResultWithSource, } from '../types.flow'; @@ -73,6 +74,12 @@ let mockedDependencyTree: Map< * a changed count reflected in a change to the transform output key. */ const files = new CountingSet(); + +/* The default mock transformer in these tests may be overridden for specific + * module paths by setting an entry in this map. + */ +let transformOverrides: Map>; + let graph: TestGraph; let options; @@ -334,6 +341,7 @@ function getMatchingContextModules(graph: Graph, filePath: string) { beforeEach(async () => { mockedDependencies = new Set(); mockedDependencyTree = new Map(); + transformOverrides = new Map(); mockTransform = jest .fn< @@ -341,6 +349,10 @@ beforeEach(async () => { Promise>, >() .mockImplementation(async (path: string, context: ?RequireContext) => { + const override = transformOverrides.get(path); + if (override != null) { + return override(path, context); + } const unstable_transformResultKey = path + (context @@ -2184,6 +2196,46 @@ describe('edge cases', () => { expect(mockTransform.mock.calls.length).toBe(4); }); + it('should not re-transform an unmodified file when it is removed and readded within a delta', async () => { + /* + ┌─────────┐ ┌──────┐ ┌───────┐ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ └──────┘ └───────┘ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await graph.initialTraverseDependencies(options); + + Actions.removeDependency('/foo', '/baz'); + Actions.addDependency('/bar', '/baz'); + + /* + ┌─────────┐ ┌──────┐ ┌───────┐ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ └──────┘ └───────┘ + │/ ┃ + /│ ┃ + ▽ ┃ + ┌──────┐ ┃ + │ /baz │ ◀━━━━━┛ + └──────┘ + */ + mockTransform.mockClear(); + expect( + getPaths(await graph.traverseDependencies([...files], options)), + ).toEqual({ + added: new Set([]), + modified: new Set(['/foo', '/bar']), + deleted: new Set([]), + }); + // /baz has not been modified, but a naive implementation might re-transform it + expect(mockTransform).not.toHaveBeenCalledWith('/baz', undefined); + }); + it('should try to transform every file only once with multiple entry points', async () => { Actions.createFile('/bundle-2'); Actions.addDependency('/bundle-2', '/foo'); @@ -3251,3 +3303,192 @@ describe('parallel edges', () => { }); }); }); + +describe('recovery from transform and resolution errors', () => { + beforeEach(() => { + transformOverrides.clear(); + }); + + test('a modified parent module is reported after a child error has been cleared', async () => { + /* + ┌─────────┐ ┌──────┐ ┌──────┐ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ └──────┘ └──────┘ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await graph.initialTraverseDependencies(options); + + class BarError extends Error {} + transformOverrides.set('/bar', () => { + throw new BarError(); + }); + Actions.modifyFile('/foo'); + Actions.modifyFile('/bar'); + + /* + ┌─────────┐ ┏━━━━━━┓ ┏━━━━━━┓ + │ /bundle │ ──▶ ┃ /foo ┃ ──▶ ┃ /bar ┃ ⚠ BarError + └─────────┘ ┗━━━━━━┛ ┗━━━━━━┛ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await expect( + graph.traverseDependencies([...files], options), + ).rejects.toThrow(BarError); + + // User fixes /bar + transformOverrides.clear(); + + // NOTE: not clearing `files`, to mimic DeltaCalculator's error behaviour. + + /* + ┌─────────┐ ┏━━━━━━┓ ┏━━━━━━┓ + │ /bundle │ ──▶ ┃ /foo ┃ ──▶ ┃ /bar ┃ + └─────────┘ ┗━━━━━━┛ ┗━━━━━━┛ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + expect( + getPaths(await graph.traverseDependencies([...files], options)), + ).toEqual({ + added: new Set(), + modified: new Set(['/foo', '/bar']), + deleted: new Set(), + }); + }); + + test('report removed dependencies after being interrupted by a transform error', async () => { + /* + ┌─────────┐ ┌──────┐ ┌──────┐ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ └──────┘ └──────┘ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await graph.initialTraverseDependencies(options); + + class BadError extends Error {} + transformOverrides.set('/bad', () => { + throw new BadError(); + }); + Actions.createFile('/bad'); + Actions.removeDependency('/bundle', '/foo'); + Actions.addDependency('/bundle', '/bad'); + + /* + ┌─────────┐ / ┌──────┐ ┌──────┐ + │ /bundle │ ───/──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ / └──────┘ └──────┘ + ┃ │ + ┃ │ + ▼ ▼ + ┏━━━━━━┓ ┌──────┐ + ┃ /bad ┃ ⚠ BadError │ /baz │ + ┗━━━━━━┛ └──────┘ + */ + await expect( + graph.traverseDependencies([...files], options), + ).rejects.toBeInstanceOf(BadError); + + // User fixes /bad + transformOverrides.clear(); + + /* + ┌─────────┐ / ┌──────┐ ┌──────┐ + │ /bundle │ ───/──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ / └──────┘ └──────┘ + ┃ │ + ┃ │ + ▼ ▼ + ┏━━━━━━┓ ┌──────┐ + ┃ /bad ┃ │ /baz │ + ┗━━━━━━┛ └──────┘ + */ + expect( + getPaths(await graph.traverseDependencies([...files], options)), + ).toEqual({ + added: new Set(['/bad']), + modified: new Set(['/bundle']), + deleted: new Set(['/foo', '/bar', '/baz']), + }); + }); + + test('report new dependencies as added after correcting an error in their dependencies', async () => { + /* + ┌─────────┐ ┌──────┐ ┌──────┐ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ + └─────────┘ └──────┘ └──────┘ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await graph.initialTraverseDependencies(options); + + Actions.createFile('/new'); + Actions.createFile('/bad'); + Actions.addDependency('/bar', '/new'); + Actions.addDependency('/new', '/bad'); + class BadError extends Error {} + transformOverrides.set('/bad', () => { + throw new BadError(); + }); + + /* + + ┌─────────┐ ┌──────┐ ┌──────┐ ┏━━━━━━┓ ┏━━━━━━┓ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ ━━▶ ┃ /new ┃ ━━▶ ┃ /bad ┃ ⚠ BadError + └─────────┘ └──────┘ └──────┘ ┗━━━━━━┛ ┗━━━━━━┛ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + await expect( + graph.traverseDependencies([...files], options), + ).rejects.toBeInstanceOf(BadError); + + // User fixes /bad + transformOverrides.clear(); + /* + + ┌─────────┐ ┌──────┐ ┌──────┐ ┏━━━━━━┓ ┏━━━━━━┓ + │ /bundle │ ──▶ │ /foo │ ──▶ │ /bar │ ━━▶ ┃ /new ┃ ━━▶ ┃ /bad ┃ + └─────────┘ └──────┘ └──────┘ ┗━━━━━━┛ ┗━━━━━━┛ + │ + │ + ▼ + ┌──────┐ + │ /baz │ + └──────┘ + */ + expect( + getPaths(await graph.traverseDependencies([...files], options)), + ).toEqual({ + added: new Set(['/new', '/bad']), + modified: new Set(['/bar']), + deleted: new Set([]), + }); + }); +}); diff --git a/packages/metro/src/DeltaBundler/__tests__/buildSubgraph-test.js b/packages/metro/src/DeltaBundler/__tests__/buildSubgraph-test.js new file mode 100644 index 0000000000..3f4601950b --- /dev/null +++ b/packages/metro/src/DeltaBundler/__tests__/buildSubgraph-test.js @@ -0,0 +1,177 @@ +/** + * 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. + * + * @flow strict-local + * @format + */ + +import type {RequireContextParams} from '../../ModuleGraph/worker/collectDependencies'; +import type {Dependency, TransformResultDependency} from '../types.flow'; + +import {buildSubgraph} from '../buildSubgraph'; +import nullthrows from 'nullthrows'; + +const makeTransformDep = ( + name: string, + asyncType: null | 'weak' | 'async' = null, + contextParams?: RequireContextParams, +): TransformResultDependency => ({ + name, + data: {key: 'key-' + name, asyncType, locs: [], contextParams}, +}); + +describe('GraphTraversal', () => { + const transformDeps: Map< + string, + $ReadOnlyArray, + > = new Map([ + ['/bundle', [makeTransformDep('foo')]], + ['/foo', [makeTransformDep('bar'), makeTransformDep('baz')]], + ['/bar', []], + ['/baz', [makeTransformDep('qux', 'weak')]], + [ + '/entryWithContext', + [ + makeTransformDep('virtual', null, { + filter: { + pattern: 'contextMatch.*', + flags: 'i', + }, + mode: 'sync', + recursive: true, + }), + ], + ], + [ + '/virtual?ctx=af3bf59b8564d441084c02bdf04c4d662d74d3bd', + [makeTransformDep('contextMatch')], + ], + ['/contextMatch', []], + ]); + + let params; + + beforeEach(() => { + params = { + resolve: jest.fn((from, dependency) => ({ + filePath: `/${dependency.name}`, + type: 'sourceFile', + })), + transform: jest.fn(async (path, requireContext) => ({ + dependencies: nullthrows(transformDeps.get(path), path), + output: [], + getSource: () => Buffer.from('// source'), + })), + shouldTraverse: jest.fn( + (dependency: Dependency) => dependency.data.data.asyncType !== 'weak', + ), + }; + }); + + test('traverses all nodes out from /bundle, except a weak dependency', async () => { + const moduleData = await buildSubgraph( + new Set(['/bundle']), + new Map(), + params, + ); + expect([...moduleData.keys()]).toEqual(['/bundle', '/foo', '/bar', '/baz']); + expect(moduleData.get('/bundle')).toEqual({ + dependencies: new Map([ + [ + 'key-foo', + { + absolutePath: '/foo', + data: makeTransformDep('foo'), + }, + ], + ]), + getSource: expect.any(Function), + output: [], + resolvedContexts: new Map(), + }); + }); + + test('resolves context and traverses context matches', async () => { + const moduleData = await buildSubgraph( + new Set(['/entryWithContext']), + new Map(), + params, + ); + expect(params.transform).toHaveBeenCalledWith( + '/entryWithContext', + undefined, + ); + const expectedResolvedContext = { + filter: /contextMatch.*/i, + from: '/virtual', + mode: 'sync', + recursive: true, + }; + expect(params.transform).toHaveBeenCalledWith( + '/virtual?ctx=af3bf59b8564d441084c02bdf04c4d662d74d3bd', + expectedResolvedContext, + ); + expect(params.transform).toHaveBeenCalledWith('/contextMatch', undefined); + expect(params.transform).toHaveBeenCalledWith( + '/entryWithContext', + undefined, + ); + expect(moduleData).toEqual( + new Map([ + [ + '/entryWithContext', + { + dependencies: new Map([ + [ + 'key-virtual', + { + absolutePath: + '/virtual?ctx=af3bf59b8564d441084c02bdf04c4d662d74d3bd', + data: nullthrows(transformDeps.get('/entryWithContext'))[0], + }, + ], + ]), + resolvedContexts: new Map([ + ['key-virtual', expectedResolvedContext], + ]), + output: [], + getSource: expect.any(Function), + }, + ], + [ + '/contextMatch', + { + dependencies: new Map(), + resolvedContexts: new Map(), + output: [], + getSource: expect.any(Function), + }, + ], + [ + '/virtual?ctx=af3bf59b8564d441084c02bdf04c4d662d74d3bd', + { + dependencies: new Map([ + [ + 'key-contextMatch', + { + absolutePath: '/contextMatch', + data: nullthrows( + transformDeps.get( + '/virtual?ctx=af3bf59b8564d441084c02bdf04c4d662d74d3bd', + ), + )[0], + }, + ], + ]), + resolvedContexts: new Map(), + output: [], + getSource: expect.any(Function), + }, + ], + ]), + ); + }); +}); diff --git a/packages/metro/src/DeltaBundler/buildSubgraph.js b/packages/metro/src/DeltaBundler/buildSubgraph.js new file mode 100644 index 0000000000..dbbfa95ac3 --- /dev/null +++ b/packages/metro/src/DeltaBundler/buildSubgraph.js @@ -0,0 +1,154 @@ +/** + * 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. + * + * @flow strict-local + * @format + */ + +import type {RequireContext} from '../lib/contextModule'; +import type { + Dependency, + ModuleData, + ResolveFn, + TransformFn, + TransformResultDependency, +} from './types.flow'; + +import {deriveAbsolutePathFromContext} from '../lib/contextModule'; +import path from 'path'; + +type Parameters = $ReadOnly<{ + resolve: ResolveFn, + transform: TransformFn, + shouldTraverse: Dependency => boolean, +}>; + +function resolveDependencies( + parentPath: string, + dependencies: $ReadOnlyArray, + resolve: ResolveFn, +): { + dependencies: Map, + resolvedContexts: Map, +} { + const maybeResolvedDeps = new Map(); + const resolvedContexts = new Map(); + for (const dep of dependencies) { + let resolvedDep; + const key = dep.data.key; + + // `require.context` + const {contextParams} = dep.data; + if (contextParams) { + // Ensure the filepath has uniqueness applied to ensure multiple `require.context` + // statements can be used to target the same file with different properties. + const from = path.join(parentPath, '..', dep.name); + const absolutePath = deriveAbsolutePathFromContext(from, contextParams); + + const resolvedContext: RequireContext = { + from, + mode: contextParams.mode, + recursive: contextParams.recursive, + filter: new RegExp( + contextParams.filter.pattern, + contextParams.filter.flags, + ), + }; + + resolvedContexts.set(key, resolvedContext); + + resolvedDep = { + absolutePath, + data: dep, + }; + } else { + try { + resolvedDep = { + absolutePath: resolve(parentPath, dep).filePath, + data: dep, + }; + } catch (error) { + // Ignore unavailable optional dependencies. They are guarded + // with a try-catch block and will be handled during runtime. + if (dep.data.isOptional !== true) { + throw error; + } + } + } + + if (maybeResolvedDeps.has(key)) { + throw new Error( + `resolveDependencies: Found duplicate dependency key '${key}' in ${parentPath}`, + ); + } + maybeResolvedDeps.set(key, resolvedDep); + } + + const resolvedDeps = new Map(); + // Return just the dependencies we successfully resolved. + // FIXME: This has a bad bug affecting all dependencies *after* an unresolved + // optional dependency. We'll need to propagate the nulls all the way to the + // serializer and the require() runtime to keep the dependency map from being + // desynced from the contents of the module. + for (const [key, resolvedDep] of maybeResolvedDeps) { + if (resolvedDep) { + resolvedDeps.set(key, resolvedDep); + } + } + return {dependencies: resolvedDeps, resolvedContexts}; +} + +export async function buildSubgraph( + entryPaths: $ReadOnlySet, + resolvedContexts: $ReadOnlyMap, + {resolve, transform, shouldTraverse}: Parameters, +): Promise>> { + const moduleData: Map> = new Map(); + const visitedPaths: Set = new Set(); + + async function visit( + absolutePath: string, + requireContext: ?RequireContext, + ): Promise { + if (visitedPaths.has(absolutePath)) { + return; + } + visitedPaths.add(absolutePath); + const transformResult = await transform(absolutePath, requireContext); + + // Get the absolute path of all sub-dependencies (some of them could have been + // moved but maintain the same relative path). + const resolutionResult = resolveDependencies( + absolutePath, + transformResult.dependencies, + resolve, + ); + + moduleData.set(absolutePath, { + ...transformResult, + ...resolutionResult, + }); + + await Promise.all( + [...resolutionResult.dependencies] + .filter(([key, dependency]) => shouldTraverse(dependency)) + .map(([key, dependency]) => + visit( + dependency.absolutePath, + resolutionResult.resolvedContexts.get(dependency.data.data.key), + ), + ), + ); + } + + await Promise.all( + [...entryPaths].map(absolutePath => + visit(absolutePath, resolvedContexts.get(absolutePath)), + ), + ); + + return moduleData; +} diff --git a/packages/metro/src/DeltaBundler/types.flow.js b/packages/metro/src/DeltaBundler/types.flow.js index c3404078a5..481bc44bdd 100644 --- a/packages/metro/src/DeltaBundler/types.flow.js +++ b/packages/metro/src/DeltaBundler/types.flow.js @@ -70,6 +70,14 @@ export type Module = $ReadOnly<{ unstable_transformResultKey?: ?string, }>; +export type ModuleData = $ReadOnly<{ + dependencies: $ReadOnlyMap, + resolvedContexts: $ReadOnlyMap, + output: $ReadOnlyArray, + getSource: () => Buffer, + unstable_transformResultKey?: ?string, +}>; + export type Dependencies = Map>; export type ReadOnlyDependencies = $ReadOnlyMap< string, @@ -115,6 +123,12 @@ export type TransformFn = ( string, ?RequireContext, ) => Promise>; + +export type ResolveFn = ( + from: string, + dependency: TransformResultDependency, +) => BundlerResolution; + export type AllowOptionalDependenciesWithOptions = { +exclude: Array, }; @@ -128,10 +142,7 @@ export type BundlerResolution = $ReadOnly<{ }>; export type Options = { - +resolve: ( - from: string, - dependency: TransformResultDependency, - ) => BundlerResolution, + +resolve: ResolveFn, +transform: TransformFn, +transformOptions: TransformInputOptions, +onProgress: ?(numProcessed: number, total: number) => mixed,