Skip to content

Commit

Permalink
feat: pass dev option to resolver (#1134)
Browse files Browse the repository at this point in the history 
Summary:
Have first-class support for resolving based on if the process is bundling for `dev` or not. Ref https://discord.com/channels/514829729862516747/514832110595604510/1171554890456256522

This feature is essentially a built-in version of just passing `dev` as a custom resolver option.

Pull Request resolved: #1134

Test Plan: Updated the tests, defaulting to `dev: true`.

Reviewed By: huntie

Differential Revision: D51111860

Pulled By: robhogan

fbshipit-source-id: 40e7482717573489d590ad9b52bf92b9466e08ad
  • Loading branch information
@EvanBacon @facebook-github-bot
EvanBacon authored and facebook-github-bot committed Dec 21, 2023
1 parent a941ba4 commit 6adaed8
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 21 deletions.
6 changes: 5 additions & 1 deletion docs/Resolution.md
View file
Expand Up @@ -164,6 +164,10 @@ Parameters: (*context*, *moduleName*, *platform*)

The set of file extensions used to identify asset files. Defaults to [`resolver.assetExts`](./Configuration.md#assetexts).

#### `dev: boolean`

`true` if the resolution is for a development bundle, or `false` otherwise.

#### `doesFileExist: string => boolean`

Returns `true` if the file with the given path exists, or `false` otherwise.
Expand Down Expand Up @@ -299,7 +303,7 @@ 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 **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. The contents of [`dev`](#dev) and [`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 under the same `customResolverOptions`.
1 change: 1 addition & 0 deletions packages/metro-resolver/src/__tests__/utils.js
View file
Expand Up @@ -32,6 +32,7 @@ export function createResolutionContext(
{enableSymlinks}: $ReadOnly<{enableSymlinks?: boolean}> = {},
): $Diff<ResolutionContext, {originModulePath: string}> {
return {
dev: true,
allowHaste: true,
assetExts: new Set(['jpg', 'png']),
customResolverOptions: {},
Expand Down
3 changes: 3 additions & 0 deletions packages/metro-resolver/src/types.js
View file
Expand Up @@ -112,6 +112,9 @@ export type ResolutionContext = $ReadOnly<{
doesFileExist: DoesFileExist,
extraNodeModules: ?{[string]: string, ...},

/** Is resolving for a development bundle. */
dev: boolean,

/**
* Get the parsed contents of the specified `package.json` file.
*/
Expand Down
6 changes: 5 additions & 1 deletion packages/metro/src/DeltaBundler/__tests__/resolver-test.js
View file
Expand Up @@ -116,7 +116,7 @@ function dep(name: string): TransformResultDependency {
resolve: (
from: string,
dependency: TransformResultDependency,
resolverOptions?: ResolverInputOptions = {},
resolverOptions?: ResolverInputOptions = {dev: true},
options: void | {assumeFlatNodeModules: boolean},
) =>
dependencyGraph.resolveDependency(
Expand Down Expand Up @@ -2723,6 +2723,7 @@ function dep(name: string): TransformResultDependency {
});
expect(
resolver.resolve(p('/root1/dir/a.js'), dep('target'), {
dev: true,
customResolverOptions: {
foo: 'bar',
key: 'value',
Expand All @@ -2731,6 +2732,7 @@ function dep(name: string): TransformResultDependency {
).toEqual({type: 'sourceFile', filePath: p('/target1.js')});
expect(
resolver.resolve(p('/root1/dir/b.js'), dep('target'), {
dev: true,
customResolverOptions: {
// NOTE: reverse order from what we passed above
key: 'value',
Expand All @@ -2743,6 +2745,7 @@ function dep(name: string): TransformResultDependency {
resolveRequest.mockClear();
expect(
resolver.resolve(p('/root1/dir/b.js'), dep('target'), {
dev: true,
customResolverOptions: {
// NOTE: only a subset of the options passed above
foo: 'bar',
Expand All @@ -2754,6 +2757,7 @@ function dep(name: string): TransformResultDependency {
resolveRequest.mockClear();
expect(
resolver.resolve(p('/root1/dir/b.js'), dep('target'), {
dev: true,
customResolverOptions: {
something: 'else',
},
Expand Down
16 changes: 12 additions & 4 deletions packages/metro/src/__tests__/HmrServer-test.js
View file
Expand Up @@ -212,7 +212,9 @@ describe('HmrServer', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -239,7 +241,9 @@ describe('HmrServer', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -266,7 +270,9 @@ describe('HmrServer', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -293,7 +299,9 @@ describe('HmrServer', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
)}\` was not found.`;

Expand Down
2 changes: 1 addition & 1 deletion packages/metro/src/index.flow.js
View file
Expand Up @@ -458,7 +458,7 @@ exports.buildGraph = async function (
platform,
type,
},
{customResolverOptions},
{customResolverOptions, dev},
);
} finally {
bundler.end();
Expand Down
41 changes: 32 additions & 9 deletions packages/metro/src/lib/__tests__/getGraphId-test.js
View file
Expand Up @@ -30,7 +30,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
).not.toBe(
Expand All @@ -48,7 +50,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -70,7 +74,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
).not.toBe(
Expand All @@ -88,7 +94,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -110,7 +118,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
).toBe(
Expand All @@ -128,7 +138,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -154,7 +166,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
).toBe(
Expand All @@ -176,7 +190,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand All @@ -201,6 +217,7 @@ describe('getGraphId', () => {
customResolverOptions: {
foo: 'bar',
},
dev: true,
},
}),
).not.toBe(
Expand All @@ -212,6 +229,7 @@ describe('getGraphId', () => {
customResolverOptions: {
something: 'else',
},
dev: true,
},
}),
);
Expand All @@ -237,6 +255,7 @@ describe('getGraphId', () => {
a: true,
b: false,
},
dev: true,
},
}),
).toBe(
Expand All @@ -249,6 +268,7 @@ describe('getGraphId', () => {
b: false,
a: true,
},
dev: true,
},
}),
);
Expand All @@ -274,6 +294,7 @@ describe('getGraphId', () => {
unstable_allowRequireContext: false,
resolverOptions: {
customResolverOptions: undefined,
dev: true,
},
},
),
Expand All @@ -292,7 +313,9 @@ describe('getGraphId', () => {
shallow: false,
lazy: false,
unstable_allowRequireContext: false,
resolverOptions: {},
resolverOptions: {
dev: true,
},
},
),
);
Expand Down
1 change: 1 addition & 0 deletions packages/metro/src/lib/splitBundleOptions.js
View file
Expand Up @@ -21,6 +21,7 @@ function splitBundleOptions(options: BundleOptions): SplitBundleOptions {
entryFile: options.entryFile,
resolverOptions: {
customResolverOptions: options.customResolverOptions,
dev: options.dev,
},
transformOptions: {
customTransformOptions: options.customTransformOptions,
Expand Down
5 changes: 1 addition & 4 deletions packages/metro/src/node-haste/DependencyGraph.js
View file
Expand Up @@ -335,10 +335,7 @@ class DependencyGraph extends EventEmitter {

// Compound key for the resolver cache
const resolverOptionsKey =
JSON.stringify(
resolverOptions.customResolverOptions ?? {},
canonicalize,
) ?? '';
JSON.stringify(resolverOptions ?? {}, canonicalize) ?? '';
const originKey = isSensitiveToOriginFolder ? path.dirname(from) : '';
const targetKey = to;
const platformKey = platform ?? NULL_PLATFORM;
Expand Down
3 changes: 2 additions & 1 deletion packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js
View file
Expand Up @@ -120,7 +120,7 @@ class ModuleResolver<TPackage: Packageish> {
},
false,
null,
/* resolverOptions */ {},
/* resolverOptions */ {dev: false},
);
this._cachedEmptyModule = emptyModule;
}
Expand Down Expand Up @@ -157,6 +157,7 @@ class ModuleResolver<TPackage: Packageish> {
{
allowHaste,
assetExts,
dev: resolverOptions.dev,
disableHierarchicalLookup,
doesFileExist,
extraNodeModules,
Expand Down
1 change: 1 addition & 0 deletions packages/metro/src/shared/types.flow.js
View file
Expand Up @@ -65,6 +65,7 @@ export type BundleOptions = {

export type ResolverInputOptions = $ReadOnly<{
customResolverOptions?: CustomResolverOptions,
dev: boolean,
}>;

export type SerializerOptions = {
Expand Down

0 comments on commit 6adaed8

Please sign in to comment.