Skip to content

Commit

Permalink
fix: handle Windows paths correctly (#53)
Browse files Browse the repository at this point in the history
* refactor(import_map): use URI for the argument of readFromJson()
  • Loading branch information
hasundue authored Oct 23, 2023
1 parent e6e5d72 commit 9233a03
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 28 deletions.
10 changes: 7 additions & 3 deletions lib/git_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class DenoCommandStub {
}
}

function normalizePath(path: string) {
return Deno.build.os === "windows" ? path.replaceAll("/", "\\") : path;
}

describe("commitAll()", () => {
let output: { path: string; content: string }[] = [];
let updates: DependencyUpdate[];
Expand Down Expand Up @@ -111,9 +115,9 @@ describe("commitAll()", () => {
assertArrayIncludes(
DenoCommandStub.commands,
[
"git add test/fixtures/direct-import/mod.ts",
"git add " + normalizePath("test/fixtures/direct-import/mod.ts"),
'git commit -m "build(deps): update node-emoji"',
"git add test/fixtures/direct-import/mod.ts",
"git add " + normalizePath("test/fixtures/direct-import/mod.ts"),
'git commit -m "build(deps): update deno.land/x/deno_graph"',
// "git add test/fixtures/direct-import/mod.ts test/fixtures/direct-import/lib.ts",
'git commit -m "build(deps): update deno.land/std"',
Expand All @@ -134,7 +138,7 @@ describe("commitAll()", () => {
'git commit -m "build(deps): update test/fixtures/direct-import/mod.ts"',
"git add test/fixtures/direct-import/lib.ts",
'git commit -m "build(deps): update test/fixtures/direct-import/lib.ts"',
],
].map(normalizePath),
);
});
});
14 changes: 8 additions & 6 deletions lib/import_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,23 @@ const isImportMapReferrer = is.ObjectOf({

// This implementation is ridiculously inefficient, but we prefer not to reimplement the whole
// import_map module. Maybe we should rathre patch rust code of the import_map module.
async function readFromJson(url: URL): Promise<Maybe<ImportMap>> {
const data = await Deno.readTextFile(url.pathname);
if (data.length === 0) return;
async function readFromJson(specifier: URI<"file">): Promise<Maybe<ImportMap>> {
const data = await Deno.readTextFile(new URL(specifier));
if (data.length === 0) {
return;
}
const json = parseJsonc(data);
if (isImportMapReferrer(json)) {
// The json seems to be deno.json or deno.jsonc referencing an import map.
return readFromJson(new URL(json.importMap, url));
return readFromJson(URI.from(new URL(json.importMap, specifier).href));
}
if (!isImportMapJson(json)) {
// The json does not include an import map.
return undefined;
}
const inner = await parseFromJson(url, json);
const inner = await parseFromJson(specifier, json);
return {
specifier: URI.from(url.href),
specifier,
resolve(specifier, referrer) {
const resolved = inner.resolve(specifier, referrer);
if (resolved === specifier) {
Expand Down
20 changes: 10 additions & 10 deletions lib/import_map_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ describe("readFromJson()", () => {
// cleanup.defer(async () => {
// await Deno.remove(f);
// });
const importMap = await ImportMap.readFromJson(new URL(f, import.meta.url));
const importMap = await ImportMap.readFromJson(URI.from(f));
assertEquals(importMap, undefined);
await Deno.remove(f);
});
it("test/fixtures/import-map/deno.json", async () => {
const importMap = await ImportMap.readFromJson(
new URL("../test/fixtures/import-map/deno.json", import.meta.url),
URI.from("../test/fixtures/import-map/deno.json", import.meta.url),
);
assertExists(importMap);
});
it("test/fixtures/import-map-referred/import_map.json", async () => {
const importMap = await ImportMap.readFromJson(
new URL(
URI.from(
"../test/fixtures/import-map-referred/deno.json",
import.meta.url,
),
Expand All @@ -39,10 +39,10 @@ describe("readFromJson()", () => {
describe("resolve()", () => {
it("resolve specifiers in import maps", async () => {
const importMap = await ImportMap.readFromJson(
new URL("../test/fixtures/import-map/deno.json", import.meta.url),
URI.from("../test/fixtures/import-map/deno.json", import.meta.url),
);
assertExists(importMap);
const referrer = URI.from("test/fixtures/import-map/mod.ts");
const referrer = URI.from("./test/fixtures/import-map/mod.ts");
assertEquals(
importMap.resolve("std/version.ts", referrer),
{
Expand Down Expand Up @@ -70,19 +70,19 @@ describe("resolve()", () => {
assertEquals(
importMap.resolve("/lib.ts", referrer),
{
specifier: URI.from("test/fixtures/import-map/lib.ts"),
specifier: URI.from("./test/fixtures/import-map/lib.ts"),
},
);
});
it("do not resolve an url", async () => {
const importMap = await ImportMap.readFromJson(
new URL(
URI.from(
"../test/fixtures/import-map-no-resolve/deno.json",
import.meta.url,
),
);
assertExists(importMap);
const referrer = URI.from("test/fixtures/import-map-no-resolve/deps.ts");
const referrer = URI.from("./test/fixtures/import-map-no-resolve/deps.ts");
assertEquals(
importMap.resolve(
"https://deno.land/std@0.171.0/testing/asserts.ts",
Expand All @@ -93,7 +93,7 @@ describe("resolve()", () => {
});
it("resolve specifiers in a referred import map", async () => {
const importMap = await ImportMap.readFromJson(
new URL(
URI.from(
"../test/fixtures/import-map-referred/deno.json",
import.meta.url,
),
Expand All @@ -115,7 +115,7 @@ describe("resolveSimple()", () => {
let importMap: ImportMap;
beforeAll(async () => {
const maybe = await ImportMap.readFromJson(
new URL("../test/fixtures/import-map/deno.json", import.meta.url),
URI.from("../test/fixtures/import-map/deno.json", import.meta.url),
);
assertExists(maybe);
importMap = maybe;
Expand Down
2 changes: 1 addition & 1 deletion lib/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export async function collect(
await DenoGraph.ensureInit();

const importMap = options.importMap
? await ImportMap.readFromJson(new URL(URI.from(options.importMap)))
? await ImportMap.readFromJson(URI.from(options.importMap))
: undefined;

const graph = await createGraph(specifiers, {
Expand Down
2 changes: 1 addition & 1 deletion lib/update_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe("_create - with import map", () => {
let importMap: ImportMap;
beforeAll(async () => {
importMap = (await ImportMap.readFromJson(
new URL("../test/fixtures/import-map/deno.json", import.meta.url),
URI.from("../test/fixtures/import-map/deno.json", import.meta.url),
))!;
});
it("std/version.ts", async () => {
Expand Down
23 changes: 17 additions & 6 deletions lib/uri.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { isAbsolute, relative, resolve, toFileUrl } from "./std/path.ts";
import { assertEquals } from "./std/assert.ts";
import { assert, is } from "./x/unknownutil.ts";
import { Brand } from "./types.ts";

export type DefaultProtocol<Scheme extends string> = Scheme extends
Expand All @@ -15,25 +17,34 @@ export type RelativePath = Brand<string, "RelativePath">;
export type AbsolutePath = Brand<string, "AbsolutePath">;

export const URI = {
from(path: string): URI<"file"> {
/**
* Convert a path to a file URL. If the path is relative, it is resolved from the current
* working directory.
*/
from(path: string | URL, base?: string): URI<"file"> {
let url: URL;
try {
url = new URL(path);
url = new URL(path, base);
assertEquals(url.protocol, "file:");
} catch {
return toFileUrl(
assert(path, is.String);
url = toFileUrl(
isAbsolute(path) ? path : resolve(path),
).href as URI<"file">;
);
}
if (url.protocol !== "file:") {
throw new TypeError(`Invalid protocol: ${url.protocol} in ${path}`);
}
return url.href as URI<"file">;
},
get cwd(): URI<"file"> {
return URI.from(Deno.cwd());
},
relative(uri: URI<"file">): RelativePath {
return relative(Deno.cwd(), new URL(uri).pathname) as RelativePath;
return relative(URI.cwd, uri) as RelativePath;
},
absolute(uri: URI<"file">): AbsolutePath {
return resolve(new URL(uri).pathname) as AbsolutePath;
return new URL(uri).pathname as AbsolutePath;
},
ensure<S extends string>(...schemes: S[]): (uri: string) => URI<S> {
return (uri) => {
Expand Down
2 changes: 1 addition & 1 deletion lib/x/unknownutil.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { is } from "https://deno.land/x/unknownutil@v3.10.0/mod.ts";
export { assert, is } from "https://deno.land/x/unknownutil@v3.10.0/mod.ts";

0 comments on commit 9233a03

Please sign in to comment.