Skip to content

Commit

Permalink
Pass customResolverOptions into resolveRequest
Browse files Browse the repository at this point in the history
Summary:
Implements the feature proposed in microsoft/rnx-kit#1803. See also D38658170 (23af730), D38705297 (e315b97), D38707475 (f2da945).

Changelog:
* **[Feature]** Pass custom resolver options from bundle URL into `resolveRequest`.

Reviewed By: GijsWeterings

Differential Revision: D38713506

fbshipit-source-id: aebbb75a13cf60447a120b00beec081d20ffc0cf
  • Loading branch information
motiz88 authored and facebook-github-bot committed Aug 24, 2022
1 parent a74ece0 commit 623b55d
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 13 deletions.
10 changes: 8 additions & 2 deletions docs/Resolution.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ A mapping of package names to directories that is consulted after the standard l

The path to the current module, e.g. the one containing the `import` we are currently resolving.

#### `customResolverOptions: {[string]: mixed}`

Any custom options passed to the resolver. By default, Metro populates this based on URL parameters in the bundle request, e.g. `http://localhost:8081/index.bundle?resolver.key=value` becomes `{key: 'value'}`.

#### `resolveRequest: CustomResolver`

A alternative resolver function to which the current request may be delegated. Defaults to [`resolver.resolveRequest`](./Configuration.md#resolvereqeuest).
Expand All @@ -206,7 +210,9 @@ Inside a custom resolver, `resolveRequest` is set to the default resolver functi

Resolver results may be cached under the following conditions:

1. For given origin module paths _A_ and _B_ and target module name _M_, the resolution for _M_ may be reused if _A_ and _B_ are in the same directory.
1. For given origin module paths _A_ and _B_ and target module name _M_, the resolution for _M_ may be reused if **all** of the following conditions hold:
1. _A_ and _B_ are in the same directory.
2. The contents of [`customResolverOptions`](#customresolveroptions-string-mixed) are equivalent ( = serialize to JSON the same) in both calls to the resolver.
2. Any cache of resolutions must be invalidated if any file in the project has changed.

Custom resolvers must adhere to these assumptions, e.g. they may not return different resolutions for origin modules in the same directory.
Custom resolvers must adhere to these assumptions, e.g. they may not return different resolutions for origin modules in the same directory under the same `customResolverOptions`.
33 changes: 30 additions & 3 deletions packages/metro-resolver/src/__tests__/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ const FailedToResolvePathError = require('../FailedToResolvePathError');
const Resolver = require('../index');
const path = require('path');

type FileTreeNode = $ReadOnly<{
[name: string]: true | FileTreeNode,
}>;

const CONTEXT: ResolutionContext = (() => {
const fileSet = new Set();
/* $FlowFixMe[missing-local-annot] The type annotation(s) required by Flow's
* LTI update could not be added via codemod */
(function fillFileSet(fileTree, prefix: string) {
(function fillFileSet(fileTree: FileTreeNode, prefix: string) {
for (const entName in fileTree) {
const entPath = path.join(prefix, entName);
if (fileTree[entName] === true) {
Expand Down Expand Up @@ -91,6 +93,7 @@ const CONTEXT: ResolutionContext = (() => {
);
return {
allowHaste: true,
customResolverOptions: {},
disableHierarchicalLookup: false,
doesFileExist: (filePath: string) => fileSet.has(filePath),
extraNodeModules: null,
Expand Down Expand Up @@ -717,4 +720,28 @@ describe('resolveRequest', () => {
'android',
);
});

it('receives customTransformOptions', () => {
expect(
Resolver.resolve(
{...context, customTransformOptions: {key: 'value'}},
'/root/project/foo.js',
'android',
),
).toMatchInlineSnapshot(`
Object {
"type": "empty",
}
`);
expect(resolveRequest).toBeCalledTimes(1);
expect(resolveRequest).toBeCalledWith(
{
...context,
resolveRequest: Resolver.resolve,
customTransformOptions: {key: 'value'},
},
'/root/project/foo.js',
'android',
);
});
});
1 change: 1 addition & 0 deletions packages/metro-resolver/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export type ResolutionContext = $ReadOnly<{
extraNodeModules: ?{[string]: string, ...},
originModulePath: string,
resolveRequest?: ?CustomResolver,
customResolverOptions: CustomResolverOptions,
...
}>;

Expand Down
64 changes: 64 additions & 0 deletions packages/metro/src/DeltaBundler/__tests__/resolver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2335,6 +2335,70 @@ type MockFSDirContents = $ReadOnly<{

expect(resolveRequest).toHaveBeenCalledTimes(1);
});

it('forks the cache by customResolverOptions', async () => {
setMockFileSystem({
root1: {
dir: {
'a.js': '',
'b.js': '',
},
},
root2: {
dir: {
'a.js': '',
'b.js': '',
},
},
'target1.js': {},
'target2.js': {},
});
resolver = await createResolver({resolver: {resolveRequest}});

resolveRequest.mockReturnValue({
type: 'sourceFile',
filePath: p('/target1.js'),
});
expect(
resolver.resolve(p('/root1/dir/a.js'), 'target', {
customResolverOptions: {
foo: 'bar',
key: 'value',
},
}),
).toBe(p('/target1.js'));
expect(
resolver.resolve(p('/root1/dir/b.js'), 'target', {
customResolverOptions: {
// NOTE: reverse order from what we passed above
key: 'value',
foo: 'bar',
},
}),
).toBe(p('/target1.js'));
expect(resolveRequest).toHaveBeenCalledTimes(1);

resolveRequest.mockClear();
expect(
resolver.resolve(p('/root1/dir/b.js'), 'target', {
customResolverOptions: {
// NOTE: only a subset of the options passed above
foo: 'bar',
},
}),
).toBe(p('/target1.js'));
expect(resolveRequest).toHaveBeenCalledTimes(1);

resolveRequest.mockClear();
expect(
resolver.resolve(p('/root1/dir/b.js'), 'target', {
customResolverOptions: {
something: 'else',
},
}),
).toBe(p('/target1.js'));
expect(resolveRequest).toHaveBeenCalledTimes(1);
});
});
});
});
44 changes: 36 additions & 8 deletions packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type Module from './Module';

import {ModuleMap as MetroFileMapModuleMap} from 'metro-file-map';

const canonicalize = require('metro-core/src/canonicalize');
const createHasteMap = require('./DependencyGraph/createHasteMap');
const {ModuleResolver} = require('./DependencyGraph/ModuleResolution');
const ModuleCache = require('./ModuleCache');
Expand All @@ -34,7 +35,7 @@ const {DuplicateHasteCandidatesError} = MetroFileMapModuleMap;

const NULL_PLATFORM = Symbol();

function getOrCreate<T>(
function getOrCreateMap<T>(
map: Map<string | symbol, Map<string | symbol, T>>,
field: string,
): Map<string | symbol, T> {
Expand All @@ -55,8 +56,21 @@ class DependencyGraph extends EventEmitter {
_moduleMap: MetroFileMapModuleMap;
_moduleResolver: ModuleResolver<Module, Package>;
_resolutionCache: Map<
// Custom resolver options
string | symbol,
Map<string | symbol, Map<string | symbol, string>>,
Map<
// Origin folder
string | symbol,
Map<
// Dependency name
string | symbol,
Map<
// Platform
string | symbol,
string,
>,
>,
>,
>;
_readyPromise: Promise<void>;

Expand Down Expand Up @@ -264,12 +278,26 @@ class DependencyGraph extends EventEmitter {
to === '..' ||
// Preserve standard assumptions under node_modules
from.includes(path.sep + 'node_modules' + path.sep);
const mapByDirectory = getOrCreate(
this._resolutionCache,
isSensitiveToOriginFolder ? path.dirname(from) : '',

// Compound key for the resolver cache
const resolverOptionsKey =
JSON.stringify(
resolverOptions.customResolverOptions ?? {},
canonicalize,
) ?? '';
const originKey = isSensitiveToOriginFolder ? path.dirname(from) : '';
const targetKey = to;
const platformKey = platform ?? NULL_PLATFORM;

// Traverse the resolver cache, which is a tree of maps
const mapByResolverOptions = this._resolutionCache;
const mapByOrigin = getOrCreateMap(
mapByResolverOptions,
resolverOptionsKey,
);
const mapByPlatform = getOrCreate(mapByDirectory, to);
let modulePath = mapByPlatform.get(platform ?? NULL_PLATFORM);
const mapByTarget = getOrCreateMap(mapByOrigin, originKey);
const mapByPlatform = getOrCreateMap(mapByTarget, targetKey);
let modulePath = mapByPlatform.get(platformKey);

if (!modulePath) {
try {
Expand All @@ -295,7 +323,7 @@ class DependencyGraph extends EventEmitter {
}
}

mapByPlatform.set(platform ?? NULL_PLATFORM, modulePath);
mapByPlatform.set(platformKey, modulePath);
return modulePath;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class ModuleResolver<TModule: Moduleish, TPackage: Packageish> {
const result = Resolver.resolve(
{
...this._options,
customResolverOptions: resolverOptions.customResolverOptions ?? {},
originModulePath: fromModule.path,
redirectModulePath: (modulePath: string) =>
this._redirectRequire(fromModule, modulePath),
Expand Down

0 comments on commit 623b55d

Please sign in to comment.