fix: use same logic to resolve typescript in banner and server

server/src/banner.ts

@@ -1,4 +1,5 @@
 import {parseCommandLine} from './cmdline_utils';
+import {resolveTsServer} from './version_provider';
 
 /**
  * This method provides a custom implementation for the AMD loader to resolve
@@ -7,17 +8,14 @@ import {parseCommandLine} from './cmdline_utils';
  * @param cb function to invoke with resolved modules
  */
 export function define(modules: string[], cb: (...modules: any[]) => void) {
-  function resolve(packageName: string, paths: string[]) {
-    try {
-      return require.resolve(packageName, {paths});
-    } catch {
-    }
-  }
   const TSSERVER = 'typescript/lib/tsserverlibrary';
   const resolvedModules = modules.map(m => {
-    if (m === TSSERVER || m === 'typescript') {
+    if (m === 'typescript') {
+      throw new Error(`Import '${TSSERVER}' instead of 'typescript'`);
+    }
+    if (m === TSSERVER) {
       const {tsProbeLocations} = parseCommandLine(process.argv);
-      m = resolve(TSSERVER, tsProbeLocations) || TSSERVER;
+      m = resolveTsServer(tsProbeLocations).resolvedPath;
     }
     return require(m);
   });

server/src/server.ts

@@ -10,7 +10,7 @@ import {generateHelpMessage, parseCommandLine} from './cmdline_utils';
 import {createLogger} from './logger';
 import {ServerHost} from './server_host';
 import {Session} from './session';
-import {resolveWithMinMajor} from './version_provider';
+import {resolveNgLangSvc, resolveTsServer} from './version_provider';
 
 // Parse command line arguments
 const options = parseCommandLine(process.argv);
@@ -26,9 +26,8 @@ const logger = createLogger({
   logVerbosity: options.logVerbosity,
 });
 
-const {tsProbeLocations, ngProbeLocations} = options;
-const ts = resolveWithMinMajor('typescript', 3, tsProbeLocations);
-const ng = resolveWithMinMajor('@angular/language-service', 9, ngProbeLocations);
+const ts = resolveTsServer(options.tsProbeLocations);
+const ng = resolveNgLangSvc(options.ngProbeLocations);
 
 // ServerHost provides native OS functionality
 const host = new ServerHost();

server/src/tests/version_provider_spec.ts

@@ -6,23 +6,69 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {resolveWithMinMajor} from '../version_provider';
+import {resolveNgLangSvc, resolveTsServer, Version} from '../version_provider';
 
-describe('resolveWithMinMajor', () => {
+describe('Node Module Resolver', () => {
   const probeLocations = [__dirname];
 
-  it('should find typescript >= v2', () => {
-    const result = resolveWithMinMajor('typescript', 2, probeLocations);
-    expect(result.version).toBe('3.6.4');
+  it('should be able to resolve tsserver', () => {
+    const result = resolveTsServer(probeLocations);
+    expect(result).toBeDefined();
+    expect(result.resolvedPath).toMatch(/typescript\/lib\/tsserverlibrary.js$/);
   });
 
-  it('should find typescript v3', () => {
-    const result = resolveWithMinMajor('typescript', 3, probeLocations);
-    expect(result.version).toBe('3.6.4');
+  it('should be able to resolve Angular language service', () => {
+    const result = resolveNgLangSvc(probeLocations);
+    expect(result).toBeDefined();
+    expect(result.resolvedPath).toMatch(/language-service.umd.js$/);
   });
+});
+
+describe('Version', () => {
+  it('should parse version string correctly', () => {
+    const cases: Array<[string, number, number, number]> = [
+      // version string | major | minor | patch
+      ['1', 1, 0, 0],
+      ['1.2', 1, 2, 0],
+      ['1.2.3', 1, 2, 3],
+      ['9.0.0-rc.1+126.sha-0c38aae.with-local-changes', 9, 0, 0],
+    ];
+    for (const [versionStr, major, minor, patch] of cases) {
+      const v = new Version(versionStr);
+      expect(v.major).toBe(major);
+      expect(v.minor).toBe(minor);
+      expect(v.patch).toBe(patch);
+    }
+  });
+
+  it('should compare versions correctly', () => {
+    const cases: Array<[string, string, boolean]> = [
+      // lhs | rhs | result
+      ['1', '1', true],
+      ['1', '2', false],
+      ['2', '2.0', true],
+      ['2', '2.1', false],
+      ['2', '2.0.0', true],
+      ['2', '2.0.1', false],
+
+      ['1.2', '1', true],
+      ['1.2', '2', false],
+      ['2.2', '2.1', true],
+      ['2.2', '2.7', false],
+      ['3.2', '3.2.0', true],
+      ['3.2', '3.2.1', false],
 
-  it('should fail to find typescript v4', () => {
-    expect(() => resolveWithMinMajor('typescript', 4, probeLocations))
-        .toThrowError(/^Failed to resolve 'typescript'/);
+      ['1.2.3', '1', true],
+      ['1.2.3', '2', false],
+      ['2.2.3', '2.1', true],
+      ['2.2.3', '2.3', false],
+      ['3.2.3', '3.2.2', true],
+      ['3.2.3', '3.2.4', false],
+    ];
+    for (const [s1, s2, result] of cases) {
+      const v1 = new Version(s1);
+      const v2 = new Version(s2);
+      expect(v1.greaterThanOrEqual(v2)).toBe(result, `Expect ${v1} >= ${v2}`);
+    }
   });
 });

server/src/version_provider.ts

@@ -6,65 +6,136 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import * as path from 'path';
+const MIN_TS_VERSION = '3.6';
+const MIN_NG_VERSION = '9.0';
 
 /**
  * Represents a valid node module that has been successfully resolved.
  */
-export interface NodeModule {
+interface NodeModule {
   resolvedPath: string;
-  version?: string;
+  version: Version;
 }
 
-function resolve(packageName: string, paths: string[]): NodeModule|undefined {
+function resolve(packageName: string, location: string, rootPackage?: string): NodeModule|
+    undefined {
+  rootPackage = rootPackage || packageName;
   try {
-    // Here, use native NodeJS require instead of ServerHost.require because
-    // we want the full path of the resolution provided by native
-    // `require.resolve()`, which ServerHost does not provide.
-    const resolvedPath = require.resolve(`${packageName}/package.json`, {paths});
-    const packageJson = require(resolvedPath);
+    const packageJsonPath = require.resolve(`${rootPackage}/package.json`, {
+      paths: [location],
+    });
+    const packageJson = require(packageJsonPath);
+    const resolvedPath = require.resolve(packageName, {
+      paths: [location],
+    });
     return {
-      resolvedPath: path.dirname(resolvedPath),
-      version: packageJson.version,
+      resolvedPath,
+      version: new Version(packageJson.version),
     };
   } catch {
   }
 }
 
-function minVersion(nodeModule: NodeModule, minMajor: number): boolean {
-  if (!nodeModule.version) {
-    return false;
-  }
-  const [majorStr] = nodeModule.version.split('.');
-  if (!majorStr) {
-    return false;
+/**
+ * Resolve the node module with the specified `packageName` that satisfies
+ * the specified minimum version.
+ * @param packageName name of package to be resolved
+ * @param minVersionStr minimum version
+ * @param probeLocations locations to initiate node module resolution
+ * @param rootPackage location of package.json, if different from `packageName`
+ */
+function resolveWithMinVersion(
+    packageName: string, minVersionStr: string, probeLocations: string[],
+    rootPackage?: string): NodeModule {
+  if (rootPackage && !packageName.startsWith(rootPackage)) {
+    throw new Error(`${packageName} must be in the root package`);
   }
-  const major = Number(majorStr);
-  if (isNaN(major)) {
-    return false;
+  const minVersion = new Version(minVersionStr);
+  for (const location of probeLocations) {
+    const nodeModule = resolve(packageName, location, rootPackage);
+    if (nodeModule && nodeModule.version.greaterThanOrEqual(minVersion)) {
+      return nodeModule;
+    }
   }
-  return major >= minMajor;
+  throw new Error(
+      `Failed to resolve '${packageName}' with minimum version '${minVersion}' from ` +
+      JSON.stringify(probeLocations, null, 2));
 }
 
 /**
- * Resolve the node module with the specified `packageName` that satisfies
- * the specified minimum major version.
- * @param packageName
- * @param minMajor
+ * Resolve `typescript/lib/tsserverlibrary` from the given locations.
  * @param probeLocations
  */
-export function resolveWithMinMajor(
-    packageName: string, minMajor: number, probeLocations: string[]): NodeModule {
-  for (const location of probeLocations) {
-    const nodeModule = resolve(packageName, [location]);
-    if (!nodeModule) {
-      continue;
+export function resolveTsServer(probeLocations: string[]): NodeModule {
+  const tsserver = 'typescript/lib/tsserverlibrary';
+  return resolveWithMinVersion(tsserver, MIN_TS_VERSION, probeLocations, 'typescript');
+}
+
+/**
+ * Resolve `@angular/language-service` from the given locations.
+ * @param probeLocations
+ */
+export function resolveNgLangSvc(probeLocations: string[]): NodeModule {
+  const nglangsvc = '@angular/language-service';
+  return resolveWithMinVersion(nglangsvc, MIN_NG_VERSION, probeLocations);
+}
+
+/**
+ * Converts the specified string `a` to non-negative integer.
+ * Returns -1 if the result is NaN.
+ * @param a
+ */
+function parseNonNegativeInt(a: string): number {
+  // parseInt() will try to convert as many as possible leading characters that
+  // are digits. This means a string like "123abc" will be converted to 123.
+  // For our use case, this is sufficient.
+  const i = parseInt(a, 10 /* radix */);
+  return isNaN(i) ? -1 : i;
+}
+
+export class Version {
+  readonly major: number;
+  readonly minor: number;
+  readonly patch: number;
+
+  constructor(private readonly versionStr: string) {
+    const [major, minor, patch] = Version.parseVersionStr(versionStr);
+    this.major = major;
+    this.minor = minor;
+    this.patch = patch;
+  }
+
+  greaterThanOrEqual(other: Version): boolean {
+    if (this.major < other.major) {
+      return false;
     }
-    if (minVersion(nodeModule, minMajor)) {
-      return nodeModule;
+    if (this.major > other.major) {
+      return true;
     }
+    if (this.minor < other.minor) {
+      return false;
+    }
+    if (this.minor > other.minor) {
+      return true;
+    }
+    return this.patch >= other.patch;
+  }
+
+  toString(): string {
+    return this.versionStr;
+  }
+
+  /**
+   * Converts the specified `versionStr` to its number constituents. Invalid
+   * number value is represented as negative number.
+   * @param versionStr
+   */
+  static parseVersionStr(versionStr: string): [number, number, number] {
+    const [major, minor, patch] = versionStr.split('.').map(parseNonNegativeInt);
+    return [
+      major === undefined ? 0 : major,
+      minor === undefined ? 0 : minor,
+      patch === undefined ? 0 : patch,
+    ];
   }
-  throw new Error(
-      `Failed to resolve '${packageName}' with minimum major version '${minMajor}' from ` +
-      JSON.stringify(probeLocations, null, 2));
 }