jco icon indicating copy to clipboard operation
jco copied to clipboard

Question: `jco types` generating namespaces instead of modules?

Open karthik2804 opened this issue 1 year ago • 4 comments

I am using jco types to create the typings to build an SDK based on the generated code, the generated types contain namespaces instead of modules which means IDE does not provide intellisense and also tsc fails to compile without //@ts-ignore the wit imports .

I have the following wit

package local:hello;

interface logger {
  log: func(val: string);
}

world hello {
    import logger; 
    export say-hello: func(name: string);
}

Using the following command to generate the types jco types -o types <path to wit> provides the following directory structure

types
├── hello.d.ts
└── interfaces
    └── local-hello-logger.d.ts

And the contents of the files are

// hello.d.ts
import { LocalHelloLogger } from './interfaces/local-hello-logger.js';
export function sayHello(name: string): void;

// local-hello-logger.d.ts
export namespace LocalHelloLogger {
 export function log(val: string): void;
} 

And the following is the guest code

// hello.js
import * as logger from 'local:hello/logger';

export function sayHello(name) {
  logger.log(`Hello ${name}`);
}

I could get around this by manually editing the generated types to be the following

declare module "local:hello/logger" {
  export function log(val: string): void;
}

Please let me know if I am missing something here. Thanks!

karthik2804 avatar May 21 '24 08:05 karthik2804

Explicitly declaring the imports sounds like a sensible approach. The current types are primiarly for jco transpile, where the imports don't matter to the end user as they are encapsulated, so they only matter so far as they are reexported.

In this guest generation case, it definitely makes sense to generate a declare module output for all imports. It would be nice to add a new flag for this which outputs this new form. --declare-imports or similar perhaps.

guybedford avatar May 21 '24 19:05 guybedford

I just ran into this myself. I would be happy to look into contributing the change, if you're looking for help.

cdata avatar May 30 '24 21:05 cdata

@cdata no one is currently working on this, a PR would be great if you're interested in contributing.

guybedford avatar May 30 '24 22:05 guybedford

@cdata just wanted to check in if you were working on this issue. I would be happy to look into it otherwise

karthik2804 avatar Sep 04 '24 15:09 karthik2804

@guybedford Is this something that would become the default for generated types? Namespaces could work if held correctly, but modules seems like it would work better. If I'm thinking about it correctly, it would also mean that we could generate a single d.ts reference that could roll up all the modules and would mean no more manually editing tsconfig to include the path aliases?

lachieh avatar Nov 26 '24 16:11 lachieh

@lachieh thanks for starting a PR! There are two separate use cases here:

  1. For jco componentize, being able to define modules for the imports so that type checking can work for JS guest components.
  2. For jco transpile, being able to output a single types file instead of multiple.

It sounds like you're also interested in use case (1) here? But I do think defining the import types as modules should always be a flag because its not a feature normally important to the transpile use case and may in fact not match up with expected names used by transpile consumers.

Perhaps the flag should even be jco types --guest or something like that to indicate we are generating types for the guest perspective (where --host is effectively the default today).

guybedford avatar Nov 26 '24 21:11 guybedford

Yep, it was types for guest components.

Because the generated types come out as a namespace, the IDE expects that to be able to access the publish function, it should be imported through the namespace but jco componentize expects a module import.

world:

package acme:[email protected];

world component {
    export wasmcloud:messaging/[email protected];

    import wasmcloud:messaging/[email protected];
}

Gen type changed to module:

- export namespace WasmcloudMessagingConsumer {
+ export module "wasmcloud:messaging/[email protected]" {

Guest suggested import, changed to module import:

- import {WasmcloudMessagingConsumer} from 'wasmcloud:messaging/[email protected]';
+ import {publish} from 'wasmcloud:messaging/[email protected]';

lachieh avatar Nov 26 '24 21:11 lachieh

Right, then I think this makes a lot of sense as the default output for a jco types --guest or similar and this would be a huge feature to have and properly document in componentize workflows.

Feel free to ask any questions in the PR further - it would be great to get this over the line!

guybedford avatar Nov 26 '24 21:11 guybedford

Yeah, I'll update the PR to --guest.

The other tangentially related part I've noticed is that to actually get those types to work with TS, they have to be manually added to the tsconfig paths:

{
  // ...
  "compilerOptions": {
    // ...
    "paths": {
      "wasmcloud:messaging/[email protected]": [
        "./types/interfaces/wasmcloud-messaging-consumer.d.ts"
      ]
    }
  }
}

It'd be nice if there was a way to reference all the types without needing to manually configure the paths for each module. Any thoughts on how to make this happen? Even if it was an extra .d.ts file that includes a /// <reference /> comment, it'd be less manual per module.

lachieh avatar Nov 26 '24 22:11 lachieh

I'm not sure actually. Does it work if you define an index.d.ts that contains /// <reference /> comments? Ideally it should be a one liner include only yeah.

guybedford avatar Nov 26 '24 22:11 guybedford

I'll do some experimentation and report back

lachieh avatar Nov 26 '24 22:11 lachieh

Ok, after some poking this morning the answer is that, yes, we can get to a pretty nice experience with the module declarations by just including the generated folder in the project. The caveat is that the files must be ambient module declarations in that they must only include declare module statements and must not import/export anything at the top level.

From my example before, the generated types for wasmcloud:messaging/[email protected] after my PR look like this:

declare module "wasmcloud:messaging/[email protected]" {
  /**
   * Perform a request operation on a subject
   */
  export function request(subject: string, body: Uint8Array, timeoutMs: number): BrokerMessage;
  /**
   * Publish a message to a subject without awaiting a response
   */
  export function publish(msg: BrokerMessage): void;
}
import type { BrokerMessage } from './wasmcloud-messaging-types.js';
export { BrokerMessage };

Because of the last two lines, TS assumes the file is a module and the ambient module declarations are ignored.

lachieh avatar Nov 27 '24 14:11 lachieh

For reference, the experimental command is jco guest-types. Please give feedback in the form of new issues!

lachieh avatar Dec 16 '24 16:12 lachieh