Broken import bindings in generated component

[kajacx]

When wrapping a component with jco transpile so that it can run on the web, the generated import functions are wrong, and the generated ts bindings are also wrong. See this repo at the jco-wrong-import-type-and-usage tag for full example.

Steps to reproduce

1. Create a wit protocol with multiple import functions. For example, use protocol.wit:

   
   package example:protocol

world my-world { record point { x: s32, y: s32, }

import print: func(msg: string) import import-point: func(pnt: point) -> point

export run: func() export move-point: func(pnt: point) -> point }

2. Implement the protocol in Rust using the `wit-bindgen` crate, see [plugin-rust](https://github.com/kajacx/wasm-playground/tree/jco-wrong-import-type-and-usage/plugin-rust).
3. Convert the wit-protocol WASM module to a WASM component with `wasm-tools component new plugin_rust.wasm -o component.wasm`
4. Convert the WASM component back to a WASM module so that it can run in the browser with `jco transpile component.wasm --instantiation -o out-dir`

### Expected behavior

The generated typescript bindings should export an idiomatic interface with the import functions, like this:
```ts
export interface ImportObject {
  print: (msg: string): void,
  importPoint: (pnt: Point): Point,
}

And the generated js implementation should use this import object correctly, like this:

  const importPoint = imports.importPoint;
  const print = imports.print;

Actual behaviour

The generated typescript bindings look like this:

export interface ImportObject {
  print: {
    default(msg: string): void,
  },
  'import-point': {
    default(pnt: Point): Point,
  },
}

And the generated JS code looks like this:

  const importPoint = imports.default;
  const print = imports.default;

This is wrong for several reasons:

1. The import-point function name in TS types isn't converted to use proper JS naming convention, which would be importPoint.
2. The print and import-point functions in the import object should be functions, not objects containing a default function.
3. The generated JS code is flat out wrong. According to the bindings, it should be const importPoint = imports['importPoint'].default, but ideally, it should be const importPoint = imports.importPoint.
4. Even ignoring the TS bindings, the JS code can never work, since both importPoint and print are being set to the same value.

Existing workaround

A fix for the JS code can be done with inline sed, see build-plugin.sh:

sed -E -i 's/const print = imports.default;/const print = imports.print;/' out-dir/component.js
sed -E -i 's/const importPoint = imports.default;/const importPoint = imports.importPoint;/' out-dir/component.js

This expression could be generalized, and maybe similar "fixup" could be made for the TS bindings, but ideally, the generated code would be correct without a need to fix it.

Software used

OS: Windows 10 64bit jco: 0.9.3 wasm-tools: 1.0.35 wit-bindgen (crate): 0.8.0

[guybedford]

I think you're expecting this to work as if the JS ES module system didn't exist, and that's fine, but one of the goals of this project is to design an implementation that represents the ESM integration.

Specifically, in JavaScript, when instantiating a module - you must provide namespace bindings that correspond to an object of key value export pairs. This object is called the module namespace exotic object in JavaScript.

Now we can sugar this up, and have some magic that does if (typeof obj === 'function') obj = { default: obj } kind of thing internally, and that is very much on the cards to add (and even already in the explainer), but initially I'm trying to keep the implementation as direct as possible so we can focus on the core semantics before getting to oversugared. We've seen the effects of oversugaring in the JS ecosystem today with how the ESM transition has gone where bundlers did lots of automatic stuff that I am wary of repeating those kinds of mistakes by making things too easy rather than transparent against the linking model.

Resolved the code error for instantiation in https://github.com/bytecodealliance/jco/pull/104. Always happy to have further discussion on the linking model.

[guybedford]

Fix released in 0.9.4, happy to leave this open for a little for further discussion as necessary.

[kajacx]

design an implementation that represents the ESM integration.

That's cool when there are no imports and you want only one instance, but that's not my use case. I would be happy if it worked for both ways - simple "import" method with no imports, and "full" load & instantiate methods with defined import object.

However, the example here makes no sense. It says that it will improve the ergonomics, but it doesn't show anywhere how to define the import object in this new paradigm. I guess this is what you meant by exotic object in JavaScript? Too bad that code example doesn't show how to define and use that esoteric object.

and have some magic that does if (typeof obj === 'function') obj = { default: obj } kind of thing internally

I agree that that's not a good idea. Maybe to not make the change backwards-incompatible, but not on purpose.

Fix released in 0.9.4, happy to leave this open for a little for further discussion as necessary.

Thanks @guybedford , it now works. The TS bindings are the same, but the JS code is now correctly using the object as defined in the TS bindings:

  const importPoint = imports['import-point'].default;
  const print = imports.print.default;

So the problems 1 and 2 still remain. My problem with using the 'import-point' name instead of importPoint is consistency. For example, when I define a "multi word" field name like this:

  record point {
    value-x: s32,
    value-y: s32,
  }

The TS bindings use correct JS convention:

export interface Point {
  valueX: number,
  valueY: number,
}

And the JS code is also correct:

  const {valueX: v0_0, valueY: v0_1 } = arg0;

So it makes no sense that that the imported function would suddenly use different naming convention. For example, the exported function is also named correctly movePoint instead of 'move-point'. So the resulting API is very inconsistent.

rather than transparent against the linking model.

I'm not sure if the imported function are linked differently than the exported ones, but if it's not a problem to rename an exported function, then is shouldn't be a problem to rename an imported function either.

When it comes to the "object containing a default function", it's not the end of the world, but again, it's inconsistent. The exported function is a normal function, it's not an object containing a default function. So how comes that the imported function is suddenly an object instead? It makes very little sense to me.

[guybedford]

@kajacx the analogy is that when in --instantiation, you are doing something similar to WebAssembly.instantiate(mod, importObject) rather than just treating "imports as direct arguments". And in the ESM integration, all modules consist of two strings - the first is the module specifier, and the second is the export name.

The module specifier is therefore kebab cased as per the import { fn } from 'some-mod' convention in JavaScript. Similarly it is not possible to have a module that is a function in JavaScript - import fn from 'some-mod' is just sugar for import { default as fn } from 'some-mod'. This same sugar is what I'm referring to.

This asymmetry between imports and exports is due to the nature of the JS module system, which affects the web integration.

Now we can forget all of the above, and simply do whatever we like of course. The question is just one of goals versus ergonomics. If these integrations are not seen as first-class, they don't need to be given such first-class consideration. But if they are, they should. As I say, I suspect we will get the sugaring I described - but it's worth noting that is a complexity added to handle the disparity between the models on the web and in other environments, not a simplification.

[kajacx]

@guybedford Well, as long as the JS code does what the TS bindings says that it will do, it really doesn't matter that much. It's great that it works at all, to be honest.

I'm still curious on how you would define imports in this new "ESM" model. Again, this directly compares the old model with imports defined to the new model with no imports defined.

Does it mean that it will be impossible to define imports? If not, then why does the article not show how imports would be passed in the new EMS model when it is directly comparing the two approaches?

I will use the "load module" and "instantiate it" as two separate actions anyway, but again, I'm just curious.

[guybedford]

Source phase imports is how imports can be provided - https://github.com/WebAssembly/esm-integration/tree/main/proposals/esm-integration#source-phase-imports.

In the non-source-phase-imports case - module imports are resolved using the browsers internal resolver and import maps.

[kajacx]

I still don't understand how I would pass in a custom import function defined in the wit protocol, but that doesn't matter, the important thing is that the JS "glue" code got fixed. Thanks.