Skip to content

Conversation

cloutiertyler
Copy link
Contributor

@cloutiertyler cloutiertyler commented Aug 19, 2025

Description of Changes

Please note, much of the code changed in this PR is generated code.

This change updates the TypeScript SDK to use a new spacetimedb TypeScript library which lives under the /crates/bindings-typescript folder alongside /crates/bindings-csharp. Just like with the C# bindings library, the types for AlgebraicType and RawModuleDef are now code generated with a script in /crates/codegen/examples.

Pulling this out into a library allows us to use the same types and serialization code on both the server and the client for TypeScript modules.

In the process of making this change I also found and fixed several issues with the TypeScript code generation. Namely an issue with recursive types and an issue with the never type. I also removed any use of namespaces since those are a TypeScript only feature, and we want to have JavaScript + types so that we can use the generated code with ESBuild without TSC.

I have also improved the npm/pnpm scripts to be able to generate TypeScript code for us automatically by running pnpm generate for any place that we have to generate typescript code. Namely:

  • Quickstart module bindings
  • AlgebraicType/ModuleDef for TypeScript module library
  • Client API messages for the TypeScript API
  • TestApp module bindings

API and ABI breaking changes

IMPORTANT! This is an API breaking change for clients, as such it should be a major version change. However, I am going to see if I can shim in the old API as best as possible to make it compatible.

Notably, we were previously exporting APIs that end users do not need, and I don't think it would ever occur to them to use, namely the whole AlgebraicValue API and also the AlgebraicType API. In principle, no one should have a need for these, although it was technically possible for them to use it.

Indeed, we could potentially even just remove AlgebraicType completely from the API by directly code generating the serialization code.

Listed below are all of the BREAKING changes to the API and their effect:

  • AlgebraicType is now a structural union literal type instead of a class (nominal type), this is a consequence of generating the type with our code gen. Users did not have a reason to use AlgebraicType directly.
  • The AlgebraicValue type has been removed entirely. This was previously a class that was exported from the SDK, but very unlikely to be used by users.
  • The ProductValue type has been removed.
  • The ReducerArgsAdapter and ValueAdapter types, which were used with AlgebraicValues have been removed.
  • Generated code has changed incompatibly so users will have to regenerate code when upgrading to this version. Technically a breaking change, but pretty easy to fix.

Listed below are the non-breaking API changes:

  • I am now exporting generated product types as the default export in addition to how I was exporting them before
  • For generated sum type T, I am now exporting a TVariants namespace which has the types of all the variants for T. This was previously exposed on the namespace T, but was inaccessible in modules other than the one it was defined in because I also export a type T in addition to namespace T and in the type position T was being interpreted as a type rather than a namespace. It's unclear why TypeScript resolved it as the T namespace within the module in which is was defined previously. Anyway, since the old types were apparently unobservable to users, I've replaced them with the types in TVariants. (Open to other names for this namespace).
  • I fixed a bug where never types (sum types with no variants) were not correctly generated.

Expected complexity level and risk

  1. The most complex thing about the PR are the potential impacts to the API. I am reasonably certain, but not 100% certain that I have accounted for everything above.

Testing

  • I ran pnpm test which runs all the tests in the repo including an integration test which tests the connection to SpacetimeDB. I also fixed broken tests.

@cloutiertyler cloutiertyler changed the title Tyler/ts module test Separate out TypeScript module library from the SDK Aug 20, 2025
@bfops bfops force-pushed the tyler/ts-module-test branch 2 times, most recently from 36c204b to f032caf Compare August 20, 2025 22:08
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2025
# Description of Changes

We had weird caching issues in the C#/Unity testsuite. Somehow, they got
triggered only as of
#3181 merging, and I
have no idea why/how.

I've restored the `id` field of the checkout step (which is used by the
cache step), and this _seems_ to have fixed it.

# API and ABI breaking changes

None.

# Expected complexity level and risk

1

# Testing

- [x] It passes on this PR
- [x] It passes in a test PR that combines this change with
#3182

---------

Co-authored-by: Zeke Foppa <[email protected]>
@cloutiertyler cloutiertyler marked this pull request as ready for review August 21, 2025 20:34
@cloutiertyler cloutiertyler requested review from JulienLavocat, gefjon and jsdt and removed request for JulienLavocat August 21, 2025 20:37
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objections to the described breaking changes. I think that ideally we should save them for a minor version bump and include them in the release notes, under a heading like "Minor incompatible changes" or something. But I don't see any reason to hold them for a major version bump other than fanatical devotion to the SemVer spec.

I don't otherwise feel equipped to review this PR, and would prefer to leave it to others.

Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the .prettierignore file should not be moved without changes. The previous file was relative to the directory it was in. We should probably at least change /.github to .github in there.

@cloutiertyler
Copy link
Contributor Author

I addressed @bfops's comment.

Copy link
Contributor

@jsdt jsdt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit difficult to filter out the autogenerated noise from the diff, but this looks ok.

@bfops bfops self-requested a review September 3, 2025 16:39
Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my codeowned files LTGM!

@bfops bfops added the release-any To be landed in any release window label Sep 3, 2025
@cloutiertyler cloutiertyler added this pull request to the merge queue Sep 3, 2025
Merged via the queue into master with commit 5901fb5 Sep 3, 2025
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants