fix(edge): fix some edge cases and add more tests

Author: jonluca

lib/bundle.ts

@@ -35,14 +35,17 @@ function bundle<S extends object = JSONSchema, O extends ParserOptions<S> = Pars
   options: O,
 ) {
   // console.log('Bundling $ref pointers in %s', parser.$refs._root$Ref.path);
+  const rootScopeBase = parser.$refs._root$Ref.dynamicIdScope
+    ? getSchemaBasePath(parser.$refs._root$Ref.path!, parser.schema)
+    : parser.$refs._root$Ref.path!;
 
   // Build an inventory of all $ref pointers in the JSON Schema
   const inventory: InventoryEntry[] = [];
   crawl<S, O>(
     parser,
     "schema",
     parser.$refs._root$Ref.path + "#",
-    parser.$refs._root$Ref.path!,
+    rootScopeBase,
     parser.$refs._root$Ref.dynamicIdScope,
     "#",
     0,
@@ -96,7 +99,7 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
   const isExcludedPath = bundleOptions.excludedPathMatcher || (() => false);
 
   if (obj && typeof obj === "object" && !ArrayBuffer.isView(obj) && !isExcludedPath(pathFromRoot)) {
-    const currentScopeBase = dynamicIdScope ? getSchemaBasePath(scopeBase, obj) : scopeBase;
+    const currentScopeBase = scopeBase;
     if ($Ref.isAllowed$Ref(obj)) {
       inventory$Ref(parent, key, path, currentScopeBase, dynamicIdScope, pathFromRoot, indirections, inventory, $refs, options);
     } else {
@@ -121,14 +124,17 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
         const keyPath = Pointer.join(path, key);
         const keyPathFromRoot = Pointer.join(pathFromRoot, key);
         const value = obj[key];
+        const childScopeBase =
+          dynamicIdScope && value && typeof value === "object" && !ArrayBuffer.isView(value)
+            ? getSchemaBasePath(currentScopeBase, value)
+            : currentScopeBase;
 
         if ($Ref.isAllowed$Ref(value)) {
-          const valueScopeBase = dynamicIdScope ? getSchemaBasePath(currentScopeBase, value) : currentScopeBase;
           inventory$Ref(
             obj,
             key,
             keyPath,
-            valueScopeBase,
+            childScopeBase,
             dynamicIdScope,
             keyPathFromRoot,
             indirections,
@@ -137,7 +143,7 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
             options,
           );
         } else {
-          crawl(obj, key, keyPath, currentScopeBase, dynamicIdScope, keyPathFromRoot, indirections, inventory, $refs, options);
+          crawl(obj, key, keyPath, childScopeBase, dynamicIdScope, keyPathFromRoot, indirections, inventory, $refs, options);
         }
 
         // We need to ensure that we have an object to work with here because we may be crawling

lib/dereference.ts

@@ -21,11 +21,14 @@ function dereference<S extends object = JSONSchema, O extends ParserOptions<S> =
   options: O,
 ) {
   const start = Date.now();
+  const rootScopeBase = parser.$refs._root$Ref.dynamicIdScope
+    ? getSchemaBasePath(parser.$refs._root$Ref.path!, parser.schema)
+    : parser.$refs._root$Ref.path!;
   // console.log('Dereferencing $ref pointers in %s', parser.$refs._root$Ref.path);
   const dereferenced = crawl<S, O>(
     parser.schema,
     parser.$refs._root$Ref.path!,
-    parser.$refs._root$Ref.path!,
+    rootScopeBase,
     parser.$refs._root$Ref.dynamicIdScope,
     "#",
     new Set(),
@@ -92,7 +95,7 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
     if (obj && typeof obj === "object" && !ArrayBuffer.isView(obj) && !isExcludedPath(pathFromRoot)) {
       parents.add(obj);
       processedObjects.add(obj);
-      const currentScopeBase = dynamicIdScope ? getSchemaBasePath(scopeBase, obj) : scopeBase;
+      const currentScopeBase = scopeBase;
 
       if ($Ref.isAllowed$Ref(obj, options)) {
         dereferenced = dereference$Ref(
@@ -123,14 +126,17 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
           }
 
           const value = obj[key];
+          const childScopeBase =
+            dynamicIdScope && value && typeof value === "object" && !ArrayBuffer.isView(value)
+              ? getSchemaBasePath(currentScopeBase, value)
+              : currentScopeBase;
           let circular;
 
           if ($Ref.isAllowed$Ref(value, options)) {
-            const valueScopeBase = dynamicIdScope ? getSchemaBasePath(currentScopeBase, value) : currentScopeBase;
             dereferenced = dereference$Ref(
               value,
               keyPath,
-              valueScopeBase,
+              childScopeBase,
               dynamicIdScope,
               keyPathFromRoot,
               parents,
@@ -183,7 +189,7 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
               dereferenced = crawl(
                 value,
                 keyPath,
-                currentScopeBase,
+                childScopeBase,
                 dynamicIdScope,
                 keyPathFromRoot,
                 parents,

lib/pointer.ts

@@ -95,7 +95,7 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
 
     // Crawl the object, one token at a time
     this.value = unwrapOrThrow(obj);
-    if (this.$ref.dynamicIdScope) {
+    if (this.$ref.dynamicIdScope && !isAliasedResource(this.$ref)) {
       this.scopeBase = getSchemaBasePath(this.scopeBase, this.value);
     }
 
@@ -196,7 +196,7 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
 
     // Crawl the object, one token at a time
     this.value = unwrapOrThrow(obj);
-    if (this.$ref.dynamicIdScope) {
+    if (this.$ref.dynamicIdScope && !isAliasedResource(this.$ref)) {
       this.scopeBase = getSchemaBasePath(this.scopeBase, this.value);
     }
 
@@ -327,18 +327,16 @@ function resolveIf$Ref<S extends object = JSONSchema, O extends ParserOptions<S>
         // This JSON reference "extends" the resolved value, rather than simply pointing to it.
         // So the resolved path does NOT change.  Just the value does.
         pointer.value = $Ref.dereference(pointer.value, resolved.value, options);
-        if (pointer.$ref.dynamicIdScope) {
-          pointer.scopeBase = getSchemaBasePath(pointer.scopeBase, pointer.value);
-        }
         return false;
       } else {
         // Resolve the reference
         pointer.$ref = resolved.$ref;
         pointer.path = resolved.path;
         pointer.value = resolved.value;
-        pointer.scopeBase = pointer.$ref.dynamicIdScope
-          ? getSchemaBasePath(pointer.$ref.path!, pointer.value)
-          : pointer.$ref.path!;
+        // `pointer.$ref.path` is already the canonical location of the resolved resource.
+        // Re-applying the resource's own `$id` here would duplicate nested path segments
+        // such as `nested/nested/foo.json`.
+        pointer.scopeBase = pointer.$ref.path!;
       }
 
       return true;
@@ -385,3 +383,7 @@ function unwrapOrThrow(value: unknown) {
 function isRootPath(pathFromRoot: string | unknown): boolean {
   return typeof pathFromRoot == "string" && Pointer.parse(pathFromRoot).length == 0;
 }
+
+function isAliasedResource($ref: $Ref<any, any>) {
+  return Boolean($ref.path && $ref.path in $ref.$refs._aliases);
+}

lib/resolve-external.ts

@@ -29,11 +29,14 @@ function resolveExternal<S extends object = JSONSchema, O extends ParserOptions<
   }
 
   try {
+    const rootScopeBase = parser.$refs._root$Ref.dynamicIdScope
+      ? getSchemaBasePath(parser.$refs._root$Ref.path!, parser.schema)
+      : parser.$refs._root$Ref.path!;
     // console.log('Resolving $ref pointers in %s', parser.$refs._root$Ref.path);
     const promises = crawl(
       parser.schema,
       parser.$refs._root$Ref.path + "#",
-      parser.$refs._root$Ref.path!,
+      rootScopeBase,
       parser.$refs._root$Ref.dynamicIdScope,
       parser.$refs,
       options,
@@ -75,7 +78,7 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
 
   if (obj && typeof obj === "object" && !ArrayBuffer.isView(obj) && !seen.has(obj)) {
     seen.add(obj); // Track previously seen objects to avoid infinite recursion
-    const currentScopeBase = dynamicIdScope ? getSchemaBasePath(scopeBase, obj) : scopeBase;
+    const currentScopeBase = scopeBase;
     if ($Ref.isExternal$Ref(obj)) {
       promises.push(resolve$Ref<S, O>(obj, path, currentScopeBase, dynamicIdScope, $refs, options));
     }
@@ -85,7 +88,9 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
       const keyPath = Pointer.join(path, key);
       const value = obj[key as keyof typeof obj] as string | JSONSchema | Buffer | undefined;
       const childScopeBase =
-        dynamicIdScope && $Ref.isExternal$Ref(value) ? getSchemaBasePath(currentScopeBase, value) : currentScopeBase;
+        dynamicIdScope && value && typeof value === "object" && !ArrayBuffer.isView(value)
+          ? getSchemaBasePath(currentScopeBase, value)
+          : currentScopeBase;
       promises = promises.concat(crawl(value, keyPath, childScopeBase, dynamicIdScope, $refs, options, seen, external));
     }
   }

lib/util/url.ts

@@ -426,6 +426,10 @@ export function fromFileSystemPath(path: string) {
  * Converts a URL to a local filesystem path.
  */
 export function toFileSystemPath(path: string | undefined, keepFileProtocol?: boolean): string {
+  // Bare "%" characters are valid in filesystem paths, but they make `decodeURI` throw.
+  // Escape only the non-encoded ones so percent-encoded sequences still decode normally.
+  path = path!.replace(/%(?![0-9A-Fa-f]{2})/g, "%25");
+
   // Step 1: `decodeURI` will decode characters such as Cyrillic characters, spaces, etc.
   path = decodeURI(path!);
 

package.json

@@ -15,7 +15,7 @@
     "test": "vitest --coverage",
     "test:specific": "vitest invalid",
     "test:node": "yarn test",
-    "test:browser": "cross-env BROWSER=\"true\" yarn test",
+    "test:browser": "node ./test/fixtures/run-browser-tests.mjs",
     "test:update": "vitest -u",
     "test:watch": "vitest -w"
   },

test/fixtures/run-browser-tests.mjs

@@ -0,0 +1,48 @@
+import { createServer } from "node:net";
+import { spawn } from "node:child_process";
+
+async function getOpenPort() {
+  const probe = createServer();
+
+  await new Promise((resolve, reject) => {
+    probe.once("error", reject);
+    probe.listen(0, "127.0.0.1", resolve);
+  });
+
+  const address = probe.address();
+  if (!address || typeof address === "string") {
+    throw new Error("Expected an ephemeral TCP port for browser tests");
+  }
+
+  await new Promise((resolve, reject) => {
+    probe.close((error) => {
+      if (error) {
+        reject(error);
+      } else {
+        resolve();
+      }
+    });
+  });
+
+  return address.port;
+}
+
+const port = await getOpenPort();
+const child = spawn(process.execPath, [".yarn/releases/yarn-4.13.0.cjs", "test"], {
+  stdio: "inherit",
+  env: {
+    ...process.env,
+    BROWSER: "true",
+    TEST_HTTP_PORT: String(port),
+    TEST_HTTP_BASE_URL: `http://localhost:${port}/`,
+  },
+});
+
+child.on("exit", (code, signal) => {
+  if (signal) {
+    process.kill(process.pid, signal);
+    return;
+  }
+
+  process.exit(code ?? 1);
+});

test/fixtures/server.ts

@@ -10,6 +10,7 @@ import path from "path";
 
 const __dirname = dirname(__filename);
 const root = path.join(__dirname, "..", "..");
+const serverPort = Number(process.env.TEST_HTTP_PORT) || 3000;
 const setup = async () => {
   const server = http
     .createServer(function (request, response) {
@@ -55,7 +56,7 @@ const setup = async () => {
         }
       });
     })
-    .listen(3000);
+    .listen(serverPort);
 
   return () => {
     // teardown