Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: symlinks in node_modules #257

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions packages/metro-resolver/src/FailedToResolveNameError.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,17 @@
const path = require('path');

class FailedToResolveNameError extends Error {
dirPaths: $ReadOnlyArray<string>;
extraPaths: $ReadOnlyArray<string>;
modulePaths: $ReadOnlyArray<string>;

constructor(
dirPaths: $ReadOnlyArray<string>,
extraPaths: $ReadOnlyArray<string>,
) {
const displayDirPaths = dirPaths.concat(extraPaths);
const hint = displayDirPaths.length ? ' or in these directories:' : '';
constructor(modulePaths: $ReadOnlyArray<string>) {
const hint = modulePaths.length ? ' or at these locations:' : '';
super(
`Module does not exist in the Haste module map${hint}\n` +
displayDirPaths
.map(dirPath => ` ${path.dirname(dirPath)}\n`)
.join(', ') +
modulePaths.map(modulePath => ` ${modulePath}\n`).join(', ') +
'\n',
);

this.dirPaths = dirPaths;
this.extraPaths = extraPaths;
this.modulePaths = modulePaths;
}
}

Expand Down
91 changes: 60 additions & 31 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,45 +90,74 @@ function resolve(
return resolution;
}
} catch (error) {}
if (isDirectImport) {
throw new Error('Failed to resolve module: ' + realModuleName);
}
}

const dirPaths = [];
for (
let currDir = path.dirname(originModulePath);
currDir !== '.' && currDir !== path.parse(originModulePath).root;
currDir = path.dirname(currDir)
) {
const searchPath = path.join(currDir, 'node_modules');
dirPaths.push(path.join(searchPath, realModuleName));
}
const modulePaths = [];
for (let modulePath of genModulePaths(context, realModuleName)) {
modulePath = context.redirectModulePath(modulePath);

const extraPaths = [];
const {extraNodeModules} = context;
if (extraNodeModules) {
let bits = path.normalize(moduleName).split(path.sep);
let packageName;
// Normalize packageName and bits for scoped modules
if (bits.length >= 2 && bits[0].startsWith('@')) {
packageName = bits.slice(0, 2).join('/');
bits = bits.slice(1);
} else {
packageName = bits[0];
}
if (extraNodeModules[packageName]) {
bits[0] = extraNodeModules[packageName];
extraPaths.push(path.join.apply(path, bits));
const result = resolveFileOrDir(context, modulePath, platform);
if (result.type === 'resolved') {
return result.resolution;
}

modulePaths.push(modulePath);
}
throw new FailedToResolveNameError(modulePaths);
}

const allDirPaths = dirPaths.concat(extraPaths);
for (let i = 0; i < allDirPaths.length; ++i) {
const realModuleName = context.redirectModulePath(allDirPaths[i]);
const result = resolveFileOrDir(context, realModuleName, platform);
if (result.type === 'resolved') {
return result.resolution;
/** Generate the potential module paths */
function* genModulePaths(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fancy

context: ResolutionContext,
toModuleName: string,
): Iterable<string> {
const {extraNodeModules, follow, originModulePath} = context;

/**
* Extract the scope and package name from the module name.
*/
let bits = path.normalize(toModuleName).split(path.sep);
let packageName, scopeName;
if (bits.length >= 2 && bits[0].startsWith('@')) {
packageName = bits.slice(0, 2).join('/');
scopeName = bits[0];
bits = bits.slice(2);
} else {
packageName = bits.shift();
}

/**
* Find the nearest "node_modules" directory that contains
* the imported package.
*/
const {root} = path.parse(originModulePath);
let parent = originModulePath;
do {
parent = path.dirname(parent);
if (path.basename(parent) !== 'node_modules') {
yield path.join(
follow(path.join(parent, 'node_modules', packageName)),
...bits,
);
}
} while (parent !== root);

/**
* Check the user-provided `extraNodeModules` module map for a
* direct mapping to a directory that contains the imported package.
*/
if (extraNodeModules) {
parent =
extraNodeModules[packageName] ||
(scopeName ? extraNodeModules[scopeName] : void 0);

if (parent) {
yield path.join(follow(path.join(parent, packageName)), ...bits);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to be path.join(parent, 'node_modules', packageName)? If not, why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous code dealing with extraNodeModules didn't do that, so I didn't either.

The idea is that a package name can be mapped to a node_modules directory it may exist in.

extraNodeModules: {
  react: '/a/b/c/node_modules'
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nevermind. I think I misread the old code.

}
}
throw new FailedToResolveNameError(dirPaths, extraPaths);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/metro-resolver/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export type FileCandidates =
*/
export type DoesFileExist = (filePath: string) => boolean;
export type IsAssetFile = (fileName: string) => boolean;
export type FollowFn = (filePath: string) => string;

/**
* Given a directory path and the base asset name, return a list of all the
Expand Down Expand Up @@ -111,6 +112,7 @@ export type ResolutionContext = ModulePathContext &
extraNodeModules: ?{[string]: string},
originModulePath: string,
resolveRequest?: ?CustomResolver,
follow: FollowFn,
};

export type CustomResolver = (
Expand Down
1 change: 1 addition & 0 deletions packages/metro/src/ModuleGraph/node-haste/node-haste.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ exports.createResolveFn = function(options: ResolveOptions): ResolveFn {
dirExists: (filePath: string): boolean => hasteFS.dirExists(filePath),
doesFileExist: (filePath: string): boolean => hasteFS.exists(filePath),
extraNodeModules,
follow: (filePath: string): string => hasteFS.follow(filePath),
isAssetFile: (filePath: string): boolean => helpers.isAssetFile(filePath),
mainFields: options.mainFields,
moduleCache,
Expand Down
7 changes: 4 additions & 3 deletions packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ class DependencyGraph extends EventEmitter {
}

_getClosestPackage(filePath: string): ?string {
const parsedPath = path.parse(filePath);
const root = parsedPath.root;
let dir = parsedPath.dir;
const {root} = path.parse(filePath);
// The `filePath` may be a directory.
let dir = filePath;
do {
const candidate = path.join(dir, 'package.json');
if (this._hasteFS.exists(candidate)) {
Expand All @@ -143,6 +143,7 @@ class DependencyGraph extends EventEmitter {

_createModuleResolver() {
this._moduleResolver = new ModuleResolver({
follow: (filePath: string) => this._hasteFS.follow(filePath),
dirExists: (filePath: string) => {
try {
return fs.lstatSync(filePath).isDirectory();
Expand Down
22 changes: 5 additions & 17 deletions packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
ResolveAsset,
} from 'metro-resolver';

export type FollowFn = (filePath: string) => string;
export type DirExistsFn = (filePath: string) => boolean;

/**
Expand Down Expand Up @@ -54,6 +55,7 @@ export type ModuleishCache<TModule, TPackage> = {
};

type Options<TModule, TPackage> = {|
+follow: FollowFn,
+dirExists: DirExistsFn,
+doesFileExist: DoesFileExist,
+extraNodeModules: ?Object,
Expand Down Expand Up @@ -173,28 +175,14 @@ class ModuleResolver<TModule: Moduleish, TPackage: Packageish> {
);
}
if (error instanceof Resolver.FailedToResolveNameError) {
const {
dirPaths,
extraPaths,
}: {
// $flowfixme these types are defined explicitly in FailedToResolveNameError but Flow refuses to recognize them here
dirPaths: $ReadOnlyArray<string>,
extraPaths: $ReadOnlyArray<string>,
} = error;
const displayDirPaths = dirPaths
.filter((dirPath: string) => this._options.dirExists(dirPath))
.map(dirPath => path.relative(this._options.projectRoot, dirPath))
.concat(extraPaths);

const hint = displayDirPaths.length ? ' or in these directories:' : '';
const {modulePaths} = error;
const hint = modulePaths.length ? ' or at these locations:' : '';
throw new UnableToResolveError(
path.relative(this._options.projectRoot, fromModule.path),
moduleName,
[
`${moduleName} could not be found within the project${hint || '.'}`,
...displayDirPaths.map(
(dirPath: string) => ` ${path.dirname(dirPath)}`,
),
...modulePaths.map(modulePath => ` ${modulePath}`),
'\nIf you are sure the module exists, try these steps:',
' 1. Clear watchman watches: watchman watch-del-all',
' 2. Delete node_modules: rm -rf node_modules and run yarn install',
Expand Down
1 change: 1 addition & 0 deletions packages/metro/src/node-haste/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// TODO(cpojer): Create a jest-types repo.
export type HasteFS = {
exists(filePath: string): boolean,
follow(filePath: string): string,
getAllFiles(): Array<string>,
getFileIterator(): Iterator<string>,
getModuleName(filePath: string): ?string,
Expand Down