Revert "Delay resolving patterns to existing versions until all final…

… versions are known. Fixes yarnpkg#3466 (yarnpkg#3477)"

This reverts commit a080a83.

The solution was still not deterministic enough (see discussion) and test was not failing before the change reliably.
Next commit comes up with a solution.

__tests__/commands/install/integration.js

@@ -822,14 +822,3 @@ test.concurrent('bailout should work with --production flag too', (): Promise<vo
     expect(await fs.exists(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js'))).toBe(false);
   });
 });
-
-test.concurrent('package version resolve should be deterministic', (): Promise<void> => {
-  // Scenario:
-  // graceful-fs will install two versions, from @4.1.10 and @^4.1.11. The pattern @^4.1.2 would sometimes resolve
-  // to 4.1.10, if @^4.1.11 hadn't been processed before. Otherwise it would resolve to the result of @^4.1.11.
-  // Run an independent install and check, and see they have different results for @^4.1.2 - won't always see
-  // the bug, but its the best we can do without creating mock registry with controlled timing of responses.
-  return runInstall({}, 'install-deterministic-versions', async (config, reporter) => {
-    await check(config, reporter, {integrity: true}, []);
-  });
-});

src/cli/commands/import.js

@@ -235,9 +235,6 @@ class ImportPackageResolver extends PackageResolver {
     deps = this.next;
     this.next = [];
     if (!deps.length) {
-      // all required package versions have been discovered, so now packages that
-      // resolved to existing versions can be resolved to their best available version
-      this.resolvePackagesWithExistingVersions();
       return;
     }
     await this.findAll(deps);

src/package-request.js

@@ -32,7 +32,6 @@ export default class PackageRequest {
     this.optional = req.optional;
     this.pattern = req.pattern;
     this.config = resolver.config;
-    this.foundInfo = null;
 
     resolver.usedRegistries.add(req.registry);
   }
@@ -55,7 +54,6 @@ export default class PackageRequest {
   config: Config;
   registry: ResolverRegistryNames;
   optional: boolean;
-  foundInfo: ?Manifest;
 
   getParentNames(): Array<string> {
     const chain = [];
@@ -228,24 +226,6 @@ export default class PackageRequest {
 
   reportResolvedRangeMatch(info: Manifest, resolved: Manifest) {}
 
-  /**
-   * Do the final resolve of a package that had a match with an existing version.
-   * After all unique versions have been discovered, so the best available version
-   * is found.
-   */
-  resolveToExistingVersion(info: Manifest) {
-    // get final resolved version
-    const {range, name} = PackageRequest.normalizePattern(this.pattern);
-    const resolved: ?Manifest = this.resolver.getHighestRangeVersionMatch(name, range);
-    invariant(resolved, 'should have a resolved reference');
-
-    this.reportResolvedRangeMatch(info, resolved);
-    const ref = resolved._reference;
-    invariant(ref, 'Resolved package info has no package reference');
-    ref.addRequest(this);
-    ref.addPattern(this.pattern, resolved);
-  }
-
   /**
    * TODO description
    */
@@ -267,7 +247,11 @@ export default class PackageRequest {
     const {range, name} = PackageRequest.normalizePattern(this.pattern);
     const resolved: ?Manifest = this.resolver.getHighestRangeVersionMatch(name, range);
     if (resolved) {
-      this.resolver.reportPackageWithExistingVersion(this, info);
+      this.reportResolvedRangeMatch(info, resolved);
+      const ref = resolved._reference;
+      invariant(ref, 'Resolved package info has no package reference');
+      ref.addRequest(this);
+      ref.addPattern(this.pattern, resolved);
       return;
     }
 

src/package-resolver.js

@@ -26,7 +26,6 @@ export default class PackageResolver {
     this.reporter = config.reporter;
     this.lockfile = lockfile;
     this.config = config;
-    this.delayedResolveQueue = [];
   }
 
   // whether the dependency graph will be flattened
@@ -71,10 +70,6 @@ export default class PackageResolver {
   // environment specific config methods and options
   config: Config;
 
-  // list of packages need to be resolved later (they found a matching version in the
-  // resolver, but better matches can still arrive later in the resolve process)
-  delayedResolveQueue: Array<{req: PackageRequest, info: Manifest}>;
-
   /**
    * TODO description
    */
@@ -457,32 +452,7 @@ export default class PackageResolver {
     const activity = (this.activity = this.reporter.activity());
     await Promise.all(deps.map((req): Promise<void> => this.find(req)));
 
-    // all required package versions have been discovered, so now packages that
-    // resolved to existing versions can be resolved to their best available version
-    this.resolvePackagesWithExistingVersions();
-
     activity.end();
     this.activity = null;
   }
-
-  /**
-    * Called by the package requester for packages that this resolver already had
-    * a matching version for. Delay the resolve, because better matches can still be
-    * discovered.
-    */
-
-  reportPackageWithExistingVersion(req: PackageRequest, info: Manifest) {
-    this.delayedResolveQueue.push({req, info});
-  }
-
-  /**
-    * Executes the resolve to existing versions for packages after the find process,
-    * when all versions that are going to be used have been discovered.
-    */
-
-  resolvePackagesWithExistingVersions() {
-    for (const {req, info} of this.delayedResolveQueue) {
-      req.resolveToExistingVersion(info);
-    }
-  }
 }