Reenable Specta integration

[oscartbeaumont]

This fixes the commented out Specta integration resolving the TODO comment.

It might be worth moving the type exporting code into the startup of the application if possible when doing a debug build so that you don't need to run cargo test ... to generate the bindings but I don't know enough about Graphite's setup to know where is best. I also suppose any Rust running in wasm would not be a good place for that as it won't have filesystem access so a better place may not exist.

TODO:

• Decide on approach
• Fix the errors with linter (we can do that from the Specta side)

Part of #1148

[Keavon]

Thank you for writing the library and helping us restore this! I'm eager to do the refactor which replaces messages.ts with these generated bindings.

[Keavon]

Back to reviewing this now! Could you please help me understand the purpose of pnpm-lock.yaml, bindings_from_node.ts, and export.js?

[oscartbeaumont]

We expose a wasm function that returns a string with all the Specta types.

This is only enabled in development so Rust can strip more for production builds.

[oscartbeaumont]

We import the wasm file and then call the get_specta_types function and save the result to the disk.

By doing this we don't need to build a separate x86/ARM binary specially for exporting the types. Commonly this would be done via cargo test. By having it be included in the binary makes it way quicker for Graphite's specific setup.

[Keavon]

I'm mostly following how this works, but still confused about when and how to use it and how it compares to the #[test] fn generate_ts_types(). How do we run this export.js file and when is that supposed to occur? The goal would be to have the bindings file be gitignored but autogenerated whenever the build server starts, or when recompiling the build server based on any change to a Rust file, which should pick up the changes to any of the Specta bindings if those structures change (without having to restart the dev server). Is the idea that each wasm binary has this bindings string enclosed and I need to add something to our Vite build process (on the web bundler side of things) to run this export.js file each time the Wasm bundle updates?

[oscartbeaumont]

I'm mostly following how this works, but still confused about when and how to use it and how it compares to the #[test] fn generate_ts_types().

Rust requires rebuilding all of the crates before they can be used to run cargo test. So the idea with exposing the bindings from the wasm file is that we only need to run a single build to get the updated bindings. This makes the whole thing much faster.

We can drop the unit tests if we go along with this. I just kept in the code so it's possible to see both possible solutions.

goal would be to have the bindings file be gitignored but autogenerated

I would generally advise against this although feel free to do whatever you think is best. I personally think having the types available in code review and when you clone in the codebase is important.

[oscartbeaumont]

This is the result of all the types that Specta generated from Graphite's codebase.

These can be used anywhere in Graphite's frontend.

[Keavon]

Shouldn't this be gitignored?

[Keavon]

And is there a way to get Specta to generate undefined instead of null in this file? We avoid null by convention.

[oscartbeaumont]

Shouldn't this be gitignored?

I would generally advise against this although feel free to do whatever you think is best. I personally think having the types available in code review and when you clone in the codebase is important.

And is there a way to get Specta to generate undefined instead of null in this file? We avoid null by convention.

I think you can use #[specta(optional)] on each of the fields.

[oscartbeaumont]

I fixed this but it's probally not needed with the wasm exporting solution (hence why it's still #[ignore]ed)

[oscartbeaumont]

I also removed the pnpm-lock.yaml. It was just from me using pnpm instead of npm.

[TrueDoctor]

It might be worth moving the type exporting code into the startup of the application if possible when doing a debug build so that you don't need to run cargo test ... to generate the bindings but I don't know enough about Graphite's setup to know where is best.

We can build the specta types in a build.rs eliminating the need to run cargo test which is pretty obscure and would probably lead to issues down the line

[oscartbeaumont]

Be aware that will require either:

• Running nested cargo builds which is not supported due to the lock on the cargo directory, or
• Splitting all of your types into a different crate than the one with the build.rs so it can import the types as a build dependency.

[TrueDoctor]

* Splitting all of your types into a different crate than the one with the `build.rs` so it can import the types as a build dependency.

Yeah, was thinking that we could put the build script in the frontend editor-wasm projects which uses the editor crate as its dependency. We thus have access to all types and have them in a separate crate

[oscartbeaumont]

That sounds great!

[TrueDoctor]

We just need to reexport the specta types static global from the editor so we can consume in then the wasm crate but that is easy to do

[Keavon]

Hi @oscartbeaumont, thanks again for all the help with this. Just an update, I finally got the chance to spend the week digging into the crazy refactors to tackle this in #3858 followed by #3865. In the latter refactor PR, I ended up researching the options that are available and discovering that Tsify is recommended by serde-wasm-bindgen (which we already use for our existing auto-generated JS->Rust entry point wrapper function declaration types) and it provided seamless integration by generating types at the same time in that file, in the same shape, every time we hit save and hot reload the project. So it ended up being the perfect fit, and for that reason I ported over the Specta attributes to Tsify attributes. However, it was still really helpful having Specta attributes tagging along for the ride these past few years, even though they weren't in use for the type generation, since they helped enforce our crate structure and made it easy to replace one for the other with them already in place. I'm elated to finally have this part of our code base cleaned up and TS type declarations generating every time we hit save, plus I've also removed every usage of any and as casting to keep our TS types safe end-to-end. So while we ended up not needing Specta starting today, I salute you for your work and personal assistance on getting us to this point. It wouldn't have happened otherwise!