fix: bug when tracing/uninstalling the base package. (#206)

src/generator.ts

@@ -834,8 +834,9 @@ export class Generator {
   async uninstall (names: string | string[]) {
     if (typeof names === 'string')
       names = [names];
-    if (this.installCnt++ === 0)
+    if (this.installCnt++ === 0) {
       this.traceMap.startInstall();
+    }
     await this.traceMap.processInputMap;
     let pins = this.traceMap.pins;
     const unusedNames = new Set([...names]);

src/install/installer.ts

@@ -151,6 +151,17 @@ export class Installer {
     return provider;
   }
 
+  /**
+   * Installs the given installation target.
+   *
+   * @param {string} pkgName Name of the package being installed.
+   * @param {InstallTarget} target The installation target being installed.
+   * @param {`./${string}` | '.'} traceSubpath
+   * @param {ResolutionMode} mode Specifies how to interact with existing installs.
+   * @param {`${string}/` | null} pkgScope URL of the package scope in which this install is occurring, null if it's a top-level install.
+   * @param {string} parentUrl URL of the parent for this install.
+   * @returns {Promise<InstalledResolution>}
+   */
   async installTarget (
     pkgName: string,
     { pkgTarget, installSubpath }: InstallTarget,
@@ -162,7 +173,7 @@ export class Installer {
     if (this.opts.freeze && mode === 'existing')
       throw new JspmError(`"${pkgName}" is not installed in the current map to freeze install, imported from ${parentUrl}.`, 'ERR_NOT_INSTALLED');
 
-    // resolutions are authoritative at the top-level
+    // Resolutions are always authoritative, and override the existing target:
     if (this.resolutions[pkgName]) {
       const resolutionTarget = newPackageTarget(this.resolutions[pkgName], this.opts.baseUrl, this.defaultRegistry, pkgName);
       resolutionTarget.installSubpath = installSubpath;
@@ -215,6 +226,16 @@ export class Installer {
     return { installUrl: pkgUrl, installSubpath };
   }
 
+  /**
+   * Installs the given package specifier.
+   *
+   * @param {string} pkgName The package specifier being installed.
+   * @param {ResolutionMode} mode Specifies how to interact with existing installs.
+   * @param {`${string}/` | null} pkgScope URL of the package scope in which this install is occurring, null if it's a top-level install.
+   * @param {`./${string}` | '.'} traceSubpath
+   * @param {string} parentUrl URL of the parent for this install.
+   * @returns {Promise<string | InstalledResolution>}
+   */
   async install (pkgName: string, mode: ResolutionMode, pkgScope: `${string}/` | null = null, traceSubpath: `./${string}` | '.', parentUrl: string = this.installBaseUrl): Promise<string | InstalledResolution> {
     if (!this.installing)
       throwInternalError('Not installing');

src/install/lock.ts

@@ -118,8 +118,6 @@ export function setConstraint (constraints: VersionConstraints, name: string, ta
 }
 
 export function setResolution (resolutions: LockResolutions, name: string, installUrl: `${string}/`, pkgScope: `${string}/` | null = null, installSubpath: `./${string}` | null = null) {
-  if (installSubpath === './std')
-    throw new Error('NEIN');
   if (pkgScope && !pkgScope.endsWith('/'))
     throwInternalError(pkgScope);
   if (pkgScope === null) {
@@ -256,16 +254,22 @@ export async function extractLockConstraintsAndMap (map: IImportMap, preloadUrls
       const targetUrl = resolveUrl(map.imports[key], mapUrl, rootUrl);
       const parsedTarget = resolver.parseUrlPkg(targetUrl);
       const pkgUrl = parsedTarget ? resolver.pkgToUrl(parsedTarget.pkg, parsedTarget.source) : await resolver.getPackageBase(targetUrl);
-      const subpath = '.' + targetUrl.slice(pkgUrl.length - 1) as '.' | `./{string}`;
+      const targetSubpath = '.' + targetUrl.slice(pkgUrl.length - 1) as '.' | `./{string}`;
+      const exportSubpath = parsedTarget && await resolver.getExportResolution(pkgUrl, targetSubpath, key);
       pkgUrls.add(pkgUrl);
-      const exportSubpath = parsedTarget && await resolver.getExportResolution(pkgUrl, subpath, key);
-      if (exportSubpath) {
+
+      // If the plain specifier resolves to a package on some provider's CDN, 
+      // and there's a corresponding export in that package's pjson, then the
+      // generator can handle this case, provided we lock the package version
+      // to match:
+      if (parsedTarget && exportSubpath) {
         // Imports resolutions that resolve as expected can be skipped
         if (key[0] === '#')
           continue;
+
         // If there is no constraint, make one as the semver major on the current version
         if (!constraints.primary[parsedKey.pkgName])
-          constraints.primary[parsedKey.pkgName] = parsedTarget ? packageTargetFromExact(parsedTarget.pkg) : pkgUrl;
+          constraints.primary[parsedKey.pkgName] = packageTargetFromExact(parsedTarget.pkg);
 
         // In the case of subpaths having diverging versions, we force convergence on one version
         // Only scopes permit unpacking
@@ -276,20 +280,33 @@ export async function extractLockConstraintsAndMap (map: IImportMap, preloadUrls
           }
           else if (exportSubpath === '.') {
             installSubpath = false;
-            // throw new Error('CASE B');
           }
-          else {
-            if (exportSubpath.endsWith(parsedKey.subpath.slice(1)))
-              installSubpath = exportSubpath.slice(0, parsedKey.subpath.length) as `./${string}/`;
+          else if (exportSubpath.endsWith(parsedKey.subpath.slice(1))) {
+            // TODO(bubblyworld): Test this, think we're using slice wrong:
+            installSubpath = exportSubpath.slice(0, parsedKey.subpath.length) as `./${string}/`;
           }
         }
         if (installSubpath !== false) {
           setResolution(lock, parsedKey.pkgName, pkgUrl, null, installSubpath);
           continue;
         }
       }
+
+      // Another possibility is that the bare specifier is a remapping for the
+      // primary package's own-name, in which case we should check whether
+      // there's a corresponding export in the primary pjson:
+      if (primaryPcfg && primaryPcfg.name === parsedKey.pkgName) {
+        const exportSubpath = await resolver.getExportResolution(primaryBase, targetSubpath, key);
+
+        // If the export subpath matches the key's subpath, then the generator
+        // can handle this case:
+        if (parsedKey.subpath === exportSubpath)
+          continue;
+      }
     }
-    // Fallback -> Custom import with normalization
+
+    // Fallback - the generator doesn't know how to resolve this bare specifier,
+    // so we need to record it as a custom import override:
     maps.imports[isPlain(key) ? key : resolveUrl(key, mapUrl, rootUrl)] = resolveUrl(map.imports[key], mapUrl, rootUrl);
   }
 

src/providers/node.ts

@@ -80,10 +80,6 @@ export async function resolveLatestTarget (target: LatestPackageTarget, layer: s
   }, layer, parentUrl);
 }
 
-// export function parsePkg (pkg: string): { pkg: ExactPackage, subpath: null | `.${string}` } {
-
-// }
-
 export function parseUrlPkg (url: string): ExactPackage | undefined {
   if (!url.startsWith('node:'))
     return;

src/trace/resolver.ts

@@ -448,6 +448,7 @@ export class Resolver {
     const env = cjsEnv ? this.cjsEnv : this.env;
     const pcfg = await this.getPackageConfig(pkgUrl) || {};
 
+    // If the package has no exports then we resolve against "node:@empty":
     if (typeof pcfg.exports === 'object' && pcfg.exports !== null && Object.keys(pcfg.exports).length === 0) {
       const stdlibTarget = { registry: 'npm', name: '@jspm/core', ranges: [new SemverRange('*')], unstable: true };
       const provider = installer.getProvider(stdlibTarget);

src/trace/tracemap.ts

@@ -106,6 +106,14 @@ export default class TraceMap {
     });
   }
 
+  /**
+   * Resolves, analyses and recursively visits the given module specifier and all of its dependencies.
+   *
+   * @param {string} specifier Module specifier to visit.
+   * @param {VisitOpts} opts Visitor configuration.
+   * @param {} parentUrl URL of the parent context for the specifier.
+   * @param {} seen Cache for optimisation.
+   */
   async visit (specifier: string, opts: VisitOpts, parentUrl = this.baseUrl.href, seen = new Set()) {
     if (!parentUrl)
       throw new Error('Internal error: expected parentUrl');

test/api/local/pkg/index.js

@@ -0,0 +1 @@
+import * as _ from './a.js';

test/api/local/pkg/package.json

@@ -1,13 +1,18 @@
 {
   "name": "localpkg",
   "exports": {
+    ".": "./index.js",
     "./custom": "./a.js",
     "./withdep": "./b.js",
     "./withdep2": "./c.js",
     "./remotedep": "./d.js",
     "./cjs": "./e.cjs",
     "./jquery": "./jquery.js",
-    "./json": "./json.ts"
+    "./json": "./json.ts",
+    "./conditional": {
+      "development": "./a.js",
+      "production": "./b.js"
+    }
   },
   "imports": {
     "#cjsdep": "./f.cjs"

test/api/self-uninstall.test.js

@@ -0,0 +1,38 @@
+import { Generator } from "@jspm/generator";
+import assert from "assert";
+
+// Mimic calling the generator from the ./local/pkg package:
+const mapUrl = new URL("./local/pkg/importmap.json", import.meta.url).href;
+const pkgNames = ["localpkg", "localpkg/custom", "localpkg/conditional"];
+
+for (const pkgName of pkgNames) {
+  let generator = new Generator({
+    mapUrl,
+    env: ["production"],
+  });
+
+  // Installing the package from within itself should resolve locally, since the
+  // package.json has a local export for ".":
+  await generator.traceInstall(pkgName);
+  let json = generator.getMap();
+  assert.ok(json.imports[pkgName]);
+
+  // Uninstalling using the same generator instance should remove the install
+  // entirely and return an empty map:
+  await generator.uninstall(pkgName);
+  assert.ok(!generator.getMap().imports);
+
+  // Uninstalling using a new generator instance should _also_ remove the install
+  // from the import map, i.e. the generator should be able to reconstruct the
+  // context given just an input map:
+  generator = new Generator({
+    mapUrl,
+    inputMap: json,
+    env: ["production"],
+  });
+
+  // Uninstalling the package should get rid of it:
+  await generator.uninstall(pkgName);
+  json = generator.getMap();
+  assert.ok(!json.imports);
+}

test/api/self.test.js

@@ -1,6 +1,12 @@
 import { Generator } from '@jspm/generator';
 import assert from 'assert';
 
+// TODO(bubblyworld): This test should be failing for two reasons:
+// 1. Generator installs take package specifiers, so self-reference installs
+//    shouldn't be possible (they're a _module_ resolution thing).
+// 2. The current version of @jspm/generator is >1.0.0, so this shouldn't
+//    be doing a local install anyway.
+
 const generator = new Generator({
   mapUrl: import.meta.url,
   env: ['source']