Make import_from_js modules serializable
#5710
Conversation
|
The generated output of |
hoodmane
force-pushed
the
hoodmane/snapshot-import-from-js
branch
from
a8d22fc to
7f5fc0a
Compare
hoodmane
force-pushed
the
hoodmane/snapshot-import-from-js
branch
from
7f5fc0a to
30efd8e
Compare
| const obj = obj_ as | ||
| | { [importName]: string; [getAccessorList]: string[] } | ||
| | undefined; | ||
| const moduleName = obj?.[importName]; |
There was a problem hiding this comment.
When obj is undefined, how about early return?
There was a problem hiding this comment.
Well we return on the next line if obj is undefined or obj[importName] is undefined.
There was a problem hiding this comment.
Yes, but if we add
if(obj === undefined) {
return
}then I guess we can make TypeScript happy and remove obj? from the rest of the function.
There was a problem hiding this comment.
I can just get rid of the ?. below since typescript knows that moduleName would be undefined if obj were nullish and so we take the early return path.
| if (!moduleName) { | ||
| return undefined; | ||
| } | ||
| modules.add(moduleName); |
There was a problem hiding this comment.
It is slightly uncomfortable to me that this function has a side effect of updating the modules set, which needs to be drilled down to this function from a very top-level makeLinearMemorySnapshot. But I guess it is trivial thing.
There was a problem hiding this comment.
Yeah it's unfortunate but the dynamic import is async and deserialization has to be async. Perhaps we should change it upstream so the deserialization can be async, that would make use cases like this simpler.
There was a problem hiding this comment.
Yeah it's unfortunate but the dynamic import is async and deserialization has to be async. Perhaps we should change it upstream so the deserialization can be async, that would make use cases like this simpler.
| if (!mod || typeof mod !== 'object') { | ||
| return mod; | ||
| } | ||
| return new Proxy(mod, { |
There was a problem hiding this comment.
This proxy if not easy to understand. Could you add some high level explanation?
3 participants
No description provided.