feat: support direct ESM imports in wasm - #55
Conversation
Directly import from an ES module following the ESM integration proposal. e.g. `(import "./add-esmi-deps.mjs" "getValue" (func $getValue (result i32)))`
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
==========================================
- Coverage 90.80% 89.02% -1.78%
==========================================
Files 6 8 +2
Lines 500 328 -172
Branches 53 68 +15
==========================================
- Hits 454 292 -162
+ Misses 45 36 -9
+ Partials 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if (pkgImport && typeof pkgImport === "string") { | ||
| importFound = true; | ||
| imports.push(genImport(pkgImport, { name: "*", as: importName })); | ||
| } else if (fs.existsSync(path.resolve(directory, moduleName))) { |
There was a problem hiding this comment.
Perhaps we could use esm resolution to support node_modules and import maps and also fail if ESM import in wasm cannot be resolved?
(we can use exsolve for this)
There was a problem hiding this comment.
Nice, I'll switch to that.
There's currently the path where imports don't have to be resolved (e.g. resolved = false). If we error out I think we'll break that. Do we want a mode where imports have to be resolved or is there some other way you are thinking we should implement that?
There was a problem hiding this comment.
We could use exsolve.resolve(id, { try: true }. Is there a case that non resolvable ids might be safe?
There was a problem hiding this comment.
You could have a non-resolved id and then supply it in the imports. e.g.
importObj = { 'someFile.js': { foo: ....} }. I don't know how useful this feature is though. I'd prefer unwasm to error out if there's an unresolvable ESM import, but I think that will break the current behavior where resolving is optional.
There was a problem hiding this comment.
Added TODO to make it throwing in next major 487fdb1
|
Hey there! Is there anything missing in this PR that stops it from being merged? |
pi0
left a comment
There was a problem hiding this comment.
Thanks and sorry for late review ❤️
Directly import from an ES module following the ESM integration proposal.
e.g.
This adds wabt as a dependency to build the test example and test case
.watfile.resolves #54