packager: ResolutionRequest: skip dirExists in loadAsDir
Summary: I'm continuing my changes to avoid `lstat`-ing the folders when we don't really need to. That changeset in particular proposes to remove the check in `_loadAsDir`. The rationale is that right after that we try for the presence of the `package.json`. If that file exists, the folder necessarily exist so we can switch these two `if` blocks for sure without changing the logic. Finally, at the end we look for the "index" file. By removing the folder check, packager would now report "file `foo/index` does not exist" rather than "directory `foo` does not exist" if the folder does not exist. I think it's not much worse, especially as both of these are unhelpful for what I believe to be a large number of cases already anyway. Indeed, the whole algo is based on a series of try/catch, and only the very last try will see its error surfaced. But, most often, it'd be useful for earlier tries to be surfaced to the user. I do want to improve that; meanwhile, however, I sense it's not a big deal to remove that folder check for all these reasons. If you don't agree, I do have another proposition: we could catch errors generated by the last `_loadAsFile` call, and rethrow them as directory errors instead (if `dirExists` return `true`). That way, the call to `dirExists` would only happen in case of errors (but that could still happen quite a lot as a result). Reviewed By: davidaurelio Differential Revision: D5028341 fbshipit-source-id: 2d4c99c0f352b71599482aa529728559466b76fd
This commit is contained in:
parent
76f74a55c2
commit
0216e87616
|
@ -569,14 +569,6 @@ class ResolutionRequest<TModule: Moduleish, TPackage: Packageish> {
|
|||
}
|
||||
|
||||
_loadAsDir(potentialDirPath: string, fromModule: TModule, toModule: string): TModule {
|
||||
if (!this._options.dirExists(potentialDirPath)) {
|
||||
throw new UnableToResolveError(
|
||||
fromModule,
|
||||
toModule,
|
||||
`Directory ${potentialDirPath} doesn't exist`,
|
||||
);
|
||||
}
|
||||
|
||||
const packageJsonPath = path.join(potentialDirPath, 'package.json');
|
||||
if (this._options.hasteFS.exists(packageJsonPath)) {
|
||||
const main = this._options.moduleCache.getPackage(packageJsonPath).getMain();
|
||||
|
|
Loading…
Reference in New Issue