Reenable Specta integration #2432

oscartbeaumont · 2025-03-14T11:19:49Z

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 · 2025-03-14T11:24:41Z

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 · 2025-05-02T01:53:05Z

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 · 2025-05-02T18:38:23Z

frontend/wasm/src/lib.rs

@@ -34,6 +34,18 @@ pub fn init_graphite() {
 	log::set_max_level(log::LevelFilter::Debug);
 }

+#[cfg(debug_assertions)]


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 · 2025-05-02T18:39:57Z

frontend/wasm/export.js

+import path from "node:path";
+import graphite, { get_specta_types } from "./pkg/graphite_wasm.js";
+
+graphite(fs.readFileSync(path.join(import.meta.dirname, './pkg/graphite_wasm_bg.wasm'))).then(() =>


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.

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?

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 · 2025-05-02T18:40:40Z

frontend/bindings_from_node.ts

@@ -0,0 +1,276 @@
+// This file has been generated by Specta. DO NOT EDIT.


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

These can be used anywhere in Graphite's frontend.

Shouldn't this be gitignored?

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

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 · 2025-05-02T18:41:16Z

editor/src/generate_ts_types.rs

@@ -1,34 +1,15 @@
 /// Running this test will generate a `types.ts` file at the root of the repo,
 /// containing every type annotated with `specta::Type`
-// #[cfg(all(test, feature = "specta-export"))]
-#[ignore]
+
 #[test]
 fn generate_ts_types() {


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

oscartbeaumont · 2025-05-02T18:42:04Z

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

TrueDoctor · 2025-05-20T07:59:18Z

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 · 2025-05-20T09:44:57Z

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 · 2025-05-20T09:50:30Z

* 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 · 2025-05-20T13:37:36Z

That sounds great!

TrueDoctor · 2025-05-21T11:30:08Z

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

Reenable Specta integration

7475f5a

oscartbeaumont added 2 commits March 14, 2025 21:13

wasm idea

bf4b9f2

improve

27756aa

Keavon force-pushed the master branch 4 times, most recently from aa7ff13 to e11b57a Compare April 6, 2025 11:41

Merge branch 'master' into reenable-specta

306f3c4

Delete pnpm-lock.yaml

54cbb5e

oscartbeaumont commented May 2, 2025

View reviewed changes

Keavon added 2 commits May 20, 2025 00:24

Merge branch 'master' into reenable-specta

711a1c7

A bit of cleanup

71a874d

		@@ -0,0 +1,276 @@
		// This file has been generated by Specta. DO NOT EDIT.

Uh oh!

Reenable Specta integration #2432

Are you sure you want to change the base?

Reenable Specta integration #2432

Uh oh!

Conversation

oscartbeaumont commented Mar 14, 2025 • edited by Keavon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Keavon commented Mar 14, 2025

Uh oh!

Keavon commented May 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oscartbeaumont commented May 2, 2025

Uh oh!

TrueDoctor commented May 20, 2025

Uh oh!

oscartbeaumont commented May 20, 2025

Uh oh!

TrueDoctor commented May 20, 2025

Uh oh!

oscartbeaumont commented May 20, 2025

Uh oh!

TrueDoctor commented May 21, 2025

Uh oh!

Uh oh!

oscartbeaumont commented Mar 14, 2025 •

edited by Keavon

Loading