Skip to content

Commit 6677fe9

Browse files
squaremosquaremo
authored
Merge pull request #2 from jkcfg/canonical_specifiers
Rough implementation of module-relative resolution
2 parents a29fb0c + 40953b9 commit 6677fe9

File tree

3 files changed

+177
-45
lines changed

3 files changed

+177
-45
lines changed

‎binding.cc

+63-34
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ struct worker_s {
3939
std::string last_exception;
4040
Persistent<Function> recv;
4141
Persistent<Context> context;
42+
// canonical path -> module
4243
std::map<std::string, Eternal<Module>> modules;
44+
// module hash -> specifier -> module
45+
std::map<int, std::map<std::string, Eternal<Module>>> resolved;
4346
};
4447

4548
// Extracts a C string from a V8 Utf8Value.
@@ -61,6 +64,41 @@ void FatalErrorCallback2(const char* location, const char* message) {
6164
}
6265
*/
6366

67+
MaybeLocal<Module> ResolveCallback(Local<Context> context,
68+
Local<String> specifier,
69+
Local<Module> referrer) {
70+
auto isolate = Isolate::GetCurrent();
71+
worker* w = (worker*)isolate->GetData(0);
72+
73+
HandleScope handle_scope(isolate);
74+
75+
String::Utf8Value str(specifier);
76+
const char* moduleName = *str;
77+
78+
if (w->resolved.count(referrer->GetIdentityHash()) == 0) {
79+
std::string out;
80+
out.append("Module (");
81+
out.append(moduleName);
82+
out.append(") has not been loaded");
83+
out.append("\n");
84+
w->last_exception = out;
85+
return MaybeLocal<Module>();
86+
}
87+
88+
std::map<std::string, Eternal<Module>> localResolve = w->resolved[referrer->GetIdentityHash()];
89+
if (localResolve.count(moduleName) == 0) {
90+
std::string out;
91+
out.append("Module (");
92+
out.append(moduleName);
93+
out.append(") has not been loaded");
94+
out.append("\n");
95+
w->last_exception = out;
96+
return MaybeLocal<Module>();
97+
}
98+
99+
return localResolve[moduleName].Get(isolate);
100+
}
101+
64102
void ExitOnPromiseRejectCallback(PromiseRejectMessage promise_reject_message) {
65103
auto isolate = Isolate::GetCurrent();
66104
worker* w = (worker*)isolate->GetData(0);
@@ -93,30 +131,6 @@ void ExitOnPromiseRejectCallback(PromiseRejectMessage promise_reject_message) {
93131
exit(1);
94132
}
95133

96-
MaybeLocal<Module> ResolveCallback(Local<Context> context,
97-
Local<String> specifier,
98-
Local<Module> referrer) {
99-
auto isolate = Isolate::GetCurrent();
100-
worker* w = (worker*)isolate->GetData(0);
101-
102-
HandleScope handle_scope(isolate);
103-
104-
String::Utf8Value str(specifier);
105-
const char* moduleName = *str;
106-
107-
if (w->modules.count(moduleName) == 0) {
108-
std::string out;
109-
out.append("Module (");
110-
out.append(moduleName);
111-
out.append(") has not been loaded");
112-
out.append("\n");
113-
w->last_exception = out;
114-
return MaybeLocal<Module>();
115-
}
116-
117-
return w->modules[moduleName].Get(isolate);
118-
}
119-
120134
// Exception details will be appended to the first argument.
121135
std::string ExceptionString(worker* w, TryCatch* try_catch) {
122136
std::string out;
@@ -224,6 +238,12 @@ int worker_load(worker* w, char* name_s, char* source_s) {
224238
}
225239

226240
int worker_load_module(worker* w, char* name_s, char* source_s, int callback_index) {
241+
// Assuming the script name is canonical, we can return immediately
242+
// if has already been loaded.
243+
if (w->modules.count(name_s) != 0) {
244+
return 0;
245+
}
246+
227247
Locker locker(w->isolate);
228248
Isolate::Scope isolate_scope(w->isolate);
229249
HandleScope handle_scope(w->isolate);
@@ -257,33 +277,42 @@ int worker_load_module(worker* w, char* name_s, char* source_s, int callback_ind
257277
return 1;
258278
}
259279

280+
// Keep track of the canonical names for resolved modules
281+
std::map<std::string, Eternal<Module>> resolved;
282+
260283
for (int i = 0; i < module->GetModuleRequestsLength(); i++) {
261284
Local<String> dependency = module->GetModuleRequest(i);
262285
String::Utf8Value str(dependency);
263286
char* dependencySpecifier = *str;
264287

265-
// If we've already loaded the module, skip resolving it.
266-
// TODO: Is there ever a time when the specifier would be the same
267-
// but would need to be resolved again?
268-
if (w->modules.count(dependencySpecifier) != 0) {
269-
continue;
270-
}
271-
272-
int ret = ResolveModule(dependencySpecifier, name_s, callback_index);
273-
if (ret != 0) {
288+
struct ResolveModule_return retval = ResolveModule(dependencySpecifier, name_s, callback_index);
289+
if (retval.r1 != 0) {
274290
// TODO: Use module->GetModuleRequestLocation() to get source locations
275291
std::string out;
276292
out.append("Module (");
277293
out.append(dependencySpecifier);
278294
out.append(") has not been loaded");
279295
out.append("\n");
280296
w->last_exception = out;
281-
return ret;
297+
return retval.r1;
298+
}
299+
// If we were successful in loading that module, it shall be
300+
// present in the modules table.
301+
if (w->modules.count(retval.r0) == 0) {
302+
std::string out;
303+
out.append("Module dependency (");
304+
out.append(retval.r0);
305+
out.append(") has not been loaded");
306+
out.append("\n");
307+
w->last_exception = out;
308+
return 1;
282309
}
310+
resolved[dependencySpecifier] = w->modules[retval.r0];
283311
}
284312

285313
Eternal<Module> persModule(w->isolate, module);
286314
w->modules[name_s] = persModule;
315+
w->resolved[module->GetIdentityHash()] = resolved;
287316

288317
Maybe<bool> ok = module->InstantiateModule(context, ResolveCallback);
289318

‎worker.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ var workerTableNextAvailable workerTableIndex = 0
5454
type ReceiveMessageCallback func(msg []byte) []byte
5555

5656
// To resolve modules from javascript.
57-
type ModuleResolverCallback func(moduleName, referrerName string) int
57+
type ModuleResolverCallback func(moduleName, referrerName string) (string, int)
5858

5959
// Don't init V8 more than once.
6060
var initV8Once sync.Once
@@ -134,7 +134,7 @@ func recvCb(buf unsafe.Pointer, buflen C.int, index workerTableIndex) C.buf {
134134
}
135135

136136
//export ResolveModule
137-
func ResolveModule(moduleSpecifier *C.char, referrerSpecifier *C.char, resolverToken int) C.int {
137+
func ResolveModule(moduleSpecifier *C.char, referrerSpecifier *C.char, resolverToken int) (*C.char, C.int) {
138138
moduleName := C.GoString(moduleSpecifier)
139139
// TODO: Remove this when I'm not dealing with Node resolution anymore
140140
referrerName := C.GoString(referrerSpecifier)
@@ -144,10 +144,11 @@ func ResolveModule(moduleSpecifier *C.char, referrerSpecifier *C.char, resolverT
144144
resolverTableLock.Unlock()
145145

146146
if resolve == nil {
147-
return C.int(1)
147+
return nil, C.int(1)
148148
}
149-
ret := resolve(moduleName, referrerName)
150-
return C.int(ret)
149+
canon, ret := resolve(moduleName, referrerName)
150+
151+
return C.CString(canon), C.int(ret)
151152
}
152153

153154
// Creates a new worker, which corresponds to a V8 isolate. A single threaded

‎worker_test.go

+108-6
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ func TestModules(t *testing.T) {
226226
err2 := worker.LoadModule("code.js", `
227227
import { test } from "dependency.js";
228228
V8Worker2.print(test);
229-
`, func(specifier string, referrer string) int {
229+
`, func(specifier string, referrer string) (string, int) {
230230
if specifier != "dependency.js" {
231231
t.Fatal(`Expected "dependency.js" specifier`)
232232
}
@@ -235,14 +235,14 @@ func TestModules(t *testing.T) {
235235
}
236236
err1 := worker.LoadModule("dependency.js", `
237237
export const test = "ready";
238-
`, func(_, _ string) int {
238+
`, func(_, _ string) (string, int) {
239239
t.Fatal(`Expected module resolver callback to not be called`)
240-
return 1
240+
return "", 1
241241
})
242242
if err1 != nil {
243243
t.Fatal(err1)
244244
}
245-
return 0
245+
return "dependency.js", 0
246246
})
247247
if err2 != nil {
248248
t.Fatal(err2)
@@ -257,15 +257,117 @@ func TestModulesMissingDependency(t *testing.T) {
257257
err := worker.LoadModule("code.js", `
258258
import { test } from "missing.js";
259259
V8Worker2.print(test);
260-
`, func(specifier string, referrer string) int {
260+
`, func(specifier string, referrer string) (string, int) {
261261
if specifier != "missing.js" {
262262
t.Fatal(`Expected "missing.js" specifier`)
263263
}
264-
return 1
264+
return "", 1
265265
})
266266
errorContains(t, err, "missing.js")
267267
}
268268

269+
func TestDuplicateSpecifiers(t *testing.T) {
270+
var worker *Worker
271+
worker = New(func(msg []byte) []byte {
272+
t.Fatal("unexpected recv")
273+
return nil
274+
})
275+
// We're testing that loading two modules, with the same
276+
// _specifier_ but different _content_ (and a different canonical
277+
// name, as returned from the callback), works OK.
278+
//
279+
// With the module loading _prior_ to using canonical specifiers,
280+
// this would fail:
281+
//
282+
// - LoadModule is called with a module name that does not match
283+
// the _requested_ specifier, which was effectively treated as a
284+
// failure to load the module
285+
// - the relative specifier `dep` in dep would be taken to refer
286+
// to `dep` itself, since the specifiers are the same, and the
287+
// export would not line up with the import.
288+
var resolver func(string, string) (string, int)
289+
resolver = func(specifier, referrer string) (string, int) {
290+
switch {
291+
case specifier == "dep" && referrer == "main":
292+
worker.LoadModule("dep", `
293+
import { nested } from 'dep';
294+
V8Worker2.print("imported dep");
295+
const value = nested;
296+
export { value };
297+
`, resolver)
298+
return "dep", 0
299+
case specifier == "dep" && referrer == "dep":
300+
worker.LoadModule("dep/dep", `
301+
const nested = 'friendship';
302+
V8Worker2.print("imported dep/dep");
303+
export { nested };
304+
`, resolver)
305+
return "dep/dep", 0
306+
}
307+
t.Fatalf("unexpected import of %q from %q", specifier, referrer)
308+
return "", 1
309+
}
310+
311+
err := worker.LoadModule("main", `
312+
import { value } from 'dep';
313+
V8Worker2.print('imported the value of', value);
314+
`, resolver)
315+
if err != nil {
316+
t.Error(err)
317+
}
318+
}
319+
320+
func TestSameModuleDifferentSpecifier(t *testing.T) {
321+
var worker *Worker
322+
worker = New(func(msg []byte) []byte {
323+
if int(msg[0]) != 2 {
324+
t.Errorf(`expected []byte{2}, got %+v`, msg)
325+
}
326+
return nil
327+
})
328+
// We're testing that loading the same module, using different
329+
// specifiers, works OK.
330+
//
331+
// With the module loading _prior_ to using canonical specifiers,
332+
// this would fail, since `dep` and `dep.js` would be assumed to
333+
// be different modules. They are assumed to be the _same_ module
334+
// here because the return value of resolve gives them the same
335+
// canonical name.
336+
var resolver func(string, string) (string, int)
337+
resolver = func(specifier, referrer string) (string, int) {
338+
339+
mod := `
340+
V8Worker2.print("imported dep.js");
341+
let counter = 0;
342+
const incr = () => { counter = counter + 1; return counter; }
343+
export { incr };
344+
`
345+
346+
switch specifier {
347+
case "dep", "dep.js":
348+
worker.LoadModule("dep.js", mod, resolver)
349+
return "dep.js", 0
350+
}
351+
t.Fatalf("unexpected import of %q from %q", specifier, referrer)
352+
return "", 1
353+
}
354+
355+
err := worker.LoadModule("main", `
356+
import { incr } from 'dep';
357+
import { incr as incr2 } from 'dep.js';
358+
359+
incr();
360+
const value = incr2();
361+
const ab = new ArrayBuffer(1);
362+
const u8 = new Uint8Array(ab);
363+
u8[0] = value;
364+
V8Worker2.send(ab);
365+
`, resolver)
366+
if err != nil {
367+
t.Error(err)
368+
}
369+
}
370+
269371
// Test breaking script execution
270372
func TestWorkerBreaking(t *testing.T) {
271373
worker := New(func(msg []byte) []byte {

0 commit comments

Comments
 (0)