-
-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support node esmodule and mf-manifest esmodule #2934
Changes from 18 commits
d9d4a22
e60daba
9cb92db
88214aa
d58b327
69d77aa
14407ec
ef194a1
5428a9a
6d21831
21b3a83
fce0a6d
caf4bd7
2542db6
30f33ea
a3e1bc5
dd254ef
ff9228d
6ba5b19
7c2df68
acc2f16
24317ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -132,7 +132,26 @@ export function createScriptNode( | |||||||
}; | ||||||||
|
||||||||
getFetch() | ||||||||
.then((f) => handleScriptFetch(f, urlObj)) | ||||||||
.then(async (f) => { | ||||||||
if (attrs?.['type'] === 'esm' || attrs?.['type'] === 'module') { | ||||||||
return loadModule(urlObj.href, { | ||||||||
fetch: f, | ||||||||
vm: await importNodeModule<typeof import('vm')>('vm'), | ||||||||
}) | ||||||||
.then(async (module) => { | ||||||||
await module.evaluate(); | ||||||||
cb(undefined, module.namespace); | ||||||||
}) | ||||||||
.catch((e) => { | ||||||||
cb( | ||||||||
e instanceof Error | ||||||||
? e | ||||||||
: new Error(`Script execution error: ${e}`), | ||||||||
); | ||||||||
}); | ||||||||
} | ||||||||
handleScriptFetch(f, urlObj); | ||||||||
}) | ||||||||
.catch((err) => { | ||||||||
cb(err); | ||||||||
}); | ||||||||
|
@@ -165,3 +184,47 @@ export function loadScriptNode( | |||||||
); | ||||||||
}); | ||||||||
} | ||||||||
|
||||||||
async function loadModule( | ||||||||
url: string, | ||||||||
options: { | ||||||||
vm: any; | ||||||||
fetch: any; | ||||||||
}, | ||||||||
parentContext?: any, | ||||||||
) { | ||||||||
const { fetch, vm } = options; | ||||||||
const context = | ||||||||
parentContext || | ||||||||
vm.createContext({ | ||||||||
...global, | ||||||||
Event, | ||||||||
URL, | ||||||||
URLSearchParams, | ||||||||
TextDecoder, | ||||||||
TextEncoder, | ||||||||
console, | ||||||||
require: eval('require'), | ||||||||
__dirname, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bundler will transform these into strings in other targets, like browser etc. cant use short hand
Suggested change
|
||||||||
__filename, | ||||||||
}); | ||||||||
const response = await fetch(url); | ||||||||
const code = await response.text(); | ||||||||
|
||||||||
const module: any = new vm.SourceTextModule(code, { | ||||||||
context, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think you should pass a custom VM context to the module. Attempting to copy the global context does not work for many cases, there are many more primitives than just
if you pass no context, then it will run in the current context and should operate like runInThisContext does for Script |
||||||||
// @ts-ignore | ||||||||
importModuleDynamically: async (specifier, script) => { | ||||||||
const resolvedUrl = new URL(specifier, url).href; | ||||||||
return loadModule(resolvedUrl, options, context); | ||||||||
}, | ||||||||
}); | ||||||||
|
||||||||
await module.link(async (specifier: string) => { | ||||||||
const resolvedUrl = new URL(specifier, url).href; | ||||||||
const module = await loadModule(resolvedUrl, options, context); | ||||||||
return module; | ||||||||
}); | ||||||||
|
||||||||
return module; | ||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider providing a default value for the
type
property to ensure it always has a valid value, even ifremoteInfo.type
is undefined:This change ensures that the
type
property will always have a value, defaulting to 'global' ifremoteInfo.type
is not provided. This can help prevent potential issues down the line if the code assumestype
is always defined.