Convert to TypeScript and ES6 modules#176
Conversation
|
This looks like a big rewrite, but most of the actual code is unchanged. |
|
@akash-joshi @graphicore @justvanrossum @lianghai Please take a look. There are many API breakages, but nothing fundamental. I took the opportunity to cleanup some (of what I think are) API warts, but you probably have more insight on the actual use of the library. This is a chance to get any API improvements we want since we are breaking API already. |
|
Since .ts files are being renamed to .ts files anyway, this is a good opportunity to move them into a src directory, so there’s a clear separation between the JS/TS source files and the build-time Wasm-generating logic. Also I wonder if the “hb” and “hbjs” files can have more explicit names, because I always find it kinda confusing to mentally navigate around the similar looking but different concepts of “harfbuzzjs”, “hbjs”, “hb”, and “harfbuzz”. For example, can hbjs.ts be either merged into index.ts, or broken down to smaller files with more specific names? Can hb.wasm/js be renamed to harfbuzz.wasm/js if they’re meant to be the Wasm build and JS wrapper (?) of harfbuzz/src/harfbuzz.cc? |
|
|
||
| // Module-level WASM state (set once by init) | ||
| let Module: any; | ||
| let exports: any; |
There was a problem hiding this comment.
If we were to use embind we could avoid these any.... maybe later.
|
Overall looks okay, my main concern is around security of the test dependencies. Then ideally it would use emscripten instead of just em++ so we get some free types and we are able to remove all |
Convert hbjs.js to hbjs.ts with full type annotations, and use tsdown to build ESM output.
Replace factory functions and separate interfaces with classes.
We were copying subarray to itself.
Now that we use a bundler, we can split source files into smaller ones. One for each class, with files for shared code. Also use unabbreviated `harfbuzz` instead of `hb` and `hbjs`.
Good idea. We didn’t use a bundler before, so a single file made sense. I have now split the source code into roughly one file per class.
I unified things on |
Can you elaborate? I thought |
| /** EmscriptenModule extended with MODULARIZE runtime methods. */ | ||
| export interface HarfBuzzModule extends EmscriptenModule { | ||
| wasmExports: Record<string, (...args: any[]) => any>; | ||
| addFunction(func: (...args: any[]) => any, signature: string): number; |
There was a problem hiding this comment.
Shouldn't -s EXTRA_EXPORTED_RUNTIME_METHODS provide these?
There was a problem hiding this comment.
This is the interface. The EmscriptenModule interface does not include these.
3 participants
No description provided.