Repository navigation
The Imports type in index.d.ts of @assemblyscript/loader is broken. #1507
Description
The problem; the type of env is not Record<string,unknown>.
/** WebAssembly imports with an optional env object and two levels of nesting. */
export type Imports = {
[key: string]: Record<string,unknown>;
env?: {
memory?: WebAssembly.Memory;
table?: WebAssembly.Table;
seed?(): number;
abort?(msg: number, file: number, line: number, column: number): void;
trace?(msg: number, numArgs?: number, ...args: number[]): void;
};
};
A suggestion; make [key:string] type any as well as Record<string,unkown> like the code below.
/** WebAssembly imports with an optional env object and two levels of nesting. */
export type Imports = {
[key: string]: Record<string,unknown> | any;
env?: {
memory?: WebAssembly.Memory;
table?: WebAssembly.Table;
seed?(): number;
abort?(msg: number, file: number, line: number, column: number): void;
trace?(msg: number, numArgs?: number, ...args: number[]): void;
};
};
Activity
[-]The Imports type in index.d.ts of @assemblyscript/loader loader is broken.[/-][+]The Imports type in index.d.ts of @assemblyscript/loader is broken.[/+]MaxGraey commentedon Oct 16, 2020
I don't see any reason for
[key: string]: Record<string,unknown> | any. It means basically[key: string]: anyand do it completely unsound. Could you provide some example which not working with existing solution?bemafred commentedon Oct 16, 2020
I hope the following explains the issue in more detail. :-)
You are right, it would be exactly the same as
[key: string]: any, but it would possibly express intention only.Below is a typescript file of mine that fails in loader/index.d.ts when compiling it with the latest tsc:
import * as fs from "fs";import * as loader from "@assemblyscript/loader";import ASModule from "../build/optimized";const imports = { /* imports go here */ };const asModule = loader.instantiateSync<typeof ASModule>(fs.readFileSync(__dirname + "/build/optimized.wasm"), imports);// TypeScript API wrappers for AssemblyScript interop logic, also a good place to manage breaking changes etc.const asm = asModule.exports;export function add(a:number, b:number): number {return asm.add(a, b);}tsc fails in loader/index.d.ts when compiling the above, reporting that
envis not of typeRecord<string,unknown>, like the indexer[key: string]: Record<string,unknown>is. The types must be the same.If I change
[key: string]: Record<string,unknown>to[key: string]: Record<string,unknown>|any;, or tokey: string]: any;it works.The reason for this is that the two lines with
envbelow are identical at runtime:const x = an_object.env;const x = an_object["env"];And the type must match the line below as well ( also at runtime ):
const y = an_object["someRandomText"];The type can not be differentiated by the runtime value of the
key: string.It only works if the types are the same, i.e. if the type is
any; which would avoid the typescript benefits regarding typing as you point out.In an untyped javascript context the structure of
Importsis ok but from a typescript context the structure should perhaps be something like the following instead (it would break the current contract):Imports {imports: { [key:string]: Record<string,unknown> }; // A typescript friendly envelope for the importsenv?: {... left out to shrink this...};}Perhaps a proper fix would be something like my experiment above but with som javascript magic to determine which contract is used by the caller? So that we do not break any code?
All the best, great work BTW!
I forked assemblyscript this morning and I have also published a package (@canyala/loader@0.15.2) that contains the typescript friendly fix
|any.It would be great to see a proper fix in the next release! :-)
MaxGraey commentedon Oct 16, 2020
So you want use own wrapper file?
Because asc could generate definitions for module via
asc .... -d ./main.d.tsAnd it should works as expected
dcodeIO commentedon Oct 16, 2020
Fwiw, I had to change
to make the error in the TS playground go away.
bemafred commentedon Oct 16, 2020
@dcodeIO Same as my problem then.
@MaxGraey Yes, I already know about the
-dflag and Iasc ...> optimized.d.tswhich gives me working intellisense when implementing my wrapper. Withoutanyorundefinedlike @dcodeIO mentioned it doesn't work. Thank you for the tip anyway. :-)The reason for the wrapper is that I want to hide wasm interop details (strings, arrays, refcounting etc.) inside my api implementation. This way I will have an isolation layer that can handle future improvements and possible breaking changes.
The wrapper layer also has access to the dom which the wasm implementation do not have (yet). My intention is to use AssemblyScript/webassembly as an optimization for specific parts of my implementation in TypeScript.
MaxGraey commentedon Oct 16, 2020
Yes, it seems, after some version of TypeScript, it became more strict about empty records. Previously, this was not a problem. @canyala Thank you for finding this! Could you use
undefinedas suggested by @dcodeIO insteadanyin your PR #1508?bemafred commentedon Oct 16, 2020
Thanks! Great! That is a reasonable explanation @MaxGraey . Yes, I'll modify my PR to use
undefined.A better choice, I agree. :-)dcodeIO commentedon Oct 20, 2020
Iiuc this issue should have been solved by #1508, but feel free to reopen if I'm mistaken :)