Skip to content

Commit 66f333f

Browse files
committed
💪 Refactor script handling to use URL type internally
Previously, the script variable in the Plugin class was always treated as a URL string, but since it was passed as a string type, there was no guarantee that the content was actually a URL. This refactoring ensures type safety by using URL type internally throughout the plugin loading process.
1 parent b0ee649 commit 66f333f

File tree

1 file changed

+27
-25
lines changed

1 file changed

+27
-25
lines changed

denops/@denops-private/plugin.ts

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import {
88
import { ensure } from "@core/unknownutil";
99
import { toFileUrl } from "@std/path/to-file-url";
1010
import { fromFileUrl } from "@std/path/from-file-url";
11-
import { join } from "@std/path/join";
12-
import { dirname } from "@std/path/dirname";
1311
import { parse as parseJsonc } from "@std/jsonc";
1412

1513
type PluginModule = {
@@ -21,14 +19,18 @@ export class Plugin {
2119
#loadedWaiter: Promise<void>;
2220
#unloadedWaiter?: Promise<void>;
2321
#disposable: AsyncDisposable = voidAsyncDisposable;
22+
#scriptUrl: URL;
2423

2524
readonly name: string;
26-
readonly script: string;
25+
26+
get script(): string {
27+
return this.#scriptUrl.href;
28+
}
2729

2830
constructor(denops: Denops, name: string, script: string) {
2931
this.#denops = denops;
3032
this.name = name;
31-
this.script = resolveScriptUrl(script);
33+
this.#scriptUrl = resolveScriptUrl(script);
3234
this.#loadedWaiter = this.#load();
3335
}
3436

@@ -39,7 +41,7 @@ export class Plugin {
3941
async #load(): Promise<void> {
4042
await emit(this.#denops, `DenopsSystemPluginPre:${this.name}`);
4143
try {
42-
const mod: PluginModule = await importPlugin(this.script);
44+
const mod: PluginModule = await importPlugin(this.#scriptUrl);
4345
this.#disposable = await mod.main(this.#denops) ?? voidAsyncDisposable;
4446
} catch (e) {
4547
// Show a warning message when Deno module cache issue is detected
@@ -110,12 +112,16 @@ const voidAsyncDisposable = {
110112

111113
const loadedScripts = new Set<string>();
112114

113-
function createScriptSuffix(script: string): string {
115+
function refreshScriptFragment(scriptUrl: URL): URL {
114116
// Import module with fragment so that reload works properly
115117
// https://github.com/vim-denops/denops.vim/issues/227
116-
const suffix = loadedScripts.has(script) ? `#${performance.now()}` : "";
117-
loadedScripts.add(script);
118-
return suffix;
118+
if (loadedScripts.has(scriptUrl.href)) {
119+
// Keep the original fragment and add a timestamp
120+
const fragment = `${scriptUrl.hash}#${performance.now()}`;
121+
return new URL(fragment, scriptUrl);
122+
}
123+
loadedScripts.add(scriptUrl.href);
124+
return scriptUrl;
119125
}
120126

121127
/** NOTE: `emit()` is never throws or rejects. */
@@ -127,11 +133,11 @@ async function emit(denops: Denops, name: string): Promise<void> {
127133
}
128134
}
129135

130-
function resolveScriptUrl(script: string): string {
136+
function resolveScriptUrl(script: string): URL {
131137
try {
132-
return toFileUrl(script).href;
138+
return toFileUrl(script);
133139
} catch {
134-
return new URL(script, import.meta.url).href;
140+
return new URL(script);
135141
}
136142
}
137143

@@ -148,9 +154,9 @@ function isDenoCacheIssueError(e: unknown): boolean {
148154
}
149155

150156
async function tryLoadImportMap(
151-
script: string,
157+
scriptUrl: URL,
152158
): Promise<ImportMap | undefined> {
153-
if (script.startsWith("http://") || script.startsWith("https://")) {
159+
if (scriptUrl.protocol !== "file:") {
154160
// We cannot load import maps for remote scripts
155161
return undefined;
156162
}
@@ -160,13 +166,9 @@ async function tryLoadImportMap(
160166
"import_map.json",
161167
"import_map.jsonc",
162168
];
163-
// Convert file URL to path for file operations
164-
const scriptPath = script.startsWith("file://")
165-
? fromFileUrl(new URL(script))
166-
: script;
167-
const parentDir = dirname(scriptPath);
168169
for (const pattern of PATTERNS) {
169-
const importMapPath = join(parentDir, pattern);
170+
const importMapUrl = new URL(pattern, scriptUrl);
171+
const importMapPath = fromFileUrl(importMapUrl);
170172
try {
171173
return await loadImportMap(importMapPath, {
172174
loader: (path: string) => {
@@ -185,13 +187,13 @@ async function tryLoadImportMap(
185187
return undefined;
186188
}
187189

188-
async function importPlugin(script: string): Promise<PluginModule> {
189-
const suffix = createScriptSuffix(script);
190-
const importMap = await tryLoadImportMap(script);
190+
async function importPlugin(scriptUrl: URL): Promise<PluginModule> {
191+
scriptUrl = refreshScriptFragment(scriptUrl);
192+
const importMap = await tryLoadImportMap(scriptUrl);
191193
if (importMap) {
192194
const importer = new ImportMapImporter(importMap);
193-
return await importer.import<PluginModule>(`${script}${suffix}`);
195+
return await importer.import<PluginModule>(scriptUrl.href);
194196
} else {
195-
return await import(`${script}${suffix}`);
197+
return await import(scriptUrl.href);
196198
}
197199
}

0 commit comments

Comments
 (0)