Simplify importing

[khaledhosny]

Do WASM initialization at module load time, so that explicit await is not needed:

import * as hb from "harfbuzzjs";
let face = new hb.Face(new hb.Blob(data));

or:

import {Blob, Face} from "harfbuzzjs";
let face = new Face(new Blob(data));

[lianghai]

Okay now I’m slowly getting to understand this project’s structure.

createHarfBuzz() in harfbuzz.d.ts should probably be typed as returning Promise<HarfBuzzModule>. Actually it may make sense to even move the definition of HarfBuzzModule from types.ts to harfbuzz.d.ts.

init() can probably just call createHarfBuzz() internally instead of accepting an argument of EmscriptenModule (or HarfBuzzModule as a result of the suggestion above).

Now I see this PR already takes advantage of top-level await in index.ts, which become available on most platforms around five years ago. If the intention is to allow users (who still can’t benefit from top-level await) to avoid importing index.ts (so they can import init() from helpers.ts and run it in an async function themselves), then this is good. If that’s not the intention, it seems like the content of init() can simply be executed directly at the top level of index.ts, with those “Module-level WASM state (set once by init)” also moved to index.ts. The question whether you want this package to be still usable on platforms that do not support top-level-await yet.

[chearon]

I don't think init is exposed right now, but it would be a good idea. Or rather, just expose the emscripten-produced createHarfbuzz. That would allow people to override the module location with Module.locateFile.

Top-level await can be problematic when using bundlers: esbuild doesn't support it, and I've had some work-around-able issues with rolldown. Rollup and webpack support it perfectly, though, as do browsers, node, bun, deno, etc. Having top-level-await by default makes sense to me since it makes the package so much easier to use.

[lianghai]

@chearon,

I don't think init is exposed right now, …

Uh, init is currently exported from helper.ts and is imported into index.ts: https://github.com/harfbuzz/harfbuzzjs/pull/184/changes#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80

Or are you talking about something else?

Having top-level-await by default makes sense to me since it makes the package so much easier to use.

Do I understand it correctly that your recommendation is to provide a simple, default API that takes advantage of top-level await (for environments that support it fine), but make sure using this package doesn’t depend on it (so, eg, esbuild and rolldown users can avoid loading the problematic module)?

[chearon]

init is currently exported

You're right, I got lost in the code. init is exported but createHarfbuzz isn't.

Do I understand it correctly

Yeah. I think this PR will work for most people, and if anyone runs into TLA issues or wants more control over the WASM request in the browser, we could export createHarfbuzz. Currently that looks easy to support.

[khaledhosny]

I don’t really care about people running ancient systems, this is generally a web-facing code where people on old systems are doing themselves a disfavor security-wise.

But if all is needed is exporting createHarfbuzz, I’m fine with that.

[khaledhosny]

I think I’m done tweaking this, is it OK now?

[lianghai]

Looks good enough – let’s move on!

A question for @chearon, not meant to block this PR: I thought createHarfbuzz has always been the default export from harfbuzz.js (and thus can be imported like import createHarfbuzz from "harfbuzzjs/harfbuzz")? Does it change anything to re-export it from index.ts besides convenience?

[lianghai]

@khaledhosny @chearon, oh I realized that export { createHarfBuzz, init }; in index.ts is actually pointless to users who can’t rely on top-level await, because they need to avoid importing index.ts in the first place (and thus need to import createHarfBuzz from harfbuss.js and init from helpers.ts).

And for users who don’t have a problem with top-level await, they don’t need export { createHarfBuzz, init };.

[khaledhosny]

@khaledhosny @chearon, oh I realized that export { createHarfBuzz, init }; in index.ts is actually pointless to users who can’t rely on top-level await, because they need to avoid importing index.ts in the first place (and thus need to import createHarfBuzz from harfbuss.js and init from helpers.ts).

And for users who don’t have a problem with top-level await, they don’t need export { createHarfBuzz, init };.

Right, I came to the same conclusion while writing the migration guide last night, but I forgot to update this PR. I’ll drop it now.

[khaledhosny]

Now, the question is what to do about init? We export it but it is not useful for no-top-level-await case, is it useful for something else (the bundlers issue?), or should I not export it as well?

[lianghai]

Uh, I thought for users who can’t depend on top-level await they can just import createHarfBuzz from harfbuss.js and init from helpers.ts and initiate the package themselves?

We export it but it is not useful for no-top-level-await case, …

Were you actually talking about the re-exporting from index.ts (feels as unnecessary as the re-exporting of createHarfBuzz there) or the original exporting from helpers.ts (feels necessary unless you want users to reimplement it)?

[khaledhosny]

Uh, I thought for users who can’t depend on top-level await they can just import createHarfBuzz from harfbuss.js and init from helpers.ts and initiate the package themselves?

But we don’t have helpers.ts in the released output, everything is bundled in one .mjs file (except for harfbuzz.js).

[lianghai]

Ah! That’s the missing piece in my understanding of this whole discussion between you and @chearon! (My understanding of bundling is very limited.) Yeah it’s tricky.

Sounds like there’s several directions:

• Start light and make the code base depend on top-level await.
  • Then the Wasm states set by init and the internal logic of init can be moved into index.ts. Later if users complain about the lack of support for those other environments, you get a concrete view of what solutions are needed by them.
• Split init and the Wasm states into a separate module, and exclude that module from bundling.
  • Again it feels the intention can be clearer if init is made an async function that awaits createHarfbuzz internally (because the whole reason for init is async/await, but the current init doesn’t show any sign of that).
  • Though I’m not familiar with the complexity of “more control over the WASM request in the browser”.
• Don’t do bundling at all.

[lianghai]

Also, is this relevant? https://tsdown.dev/recipes/wasm-support

[khaledhosny]

I’m inclined to drop the re-export of init and createHarfbuzz and do nothing else. This will make init an internal symbol, so we can tweak it without breaking the API. If someone shows up with a concrete use case, we can act accordingly.

[khaledhosny]

Also, is this relevant? https://tsdown.dev/recipes/wasm-support

I’d like to keep the separate WASM file, so people can import it manually and add whatever JS interface they like on top if it. It is a use case we always supported and advertised, and I don’t see a pressing need to break it.