Skip to content

Disabled codegen for private tables by default#4197

Open
JasonAtClockwork wants to merge 21 commits intomasterfrom
jlarabie/2.0-private-tables
Open

Disabled codegen for private tables by default#4197
JasonAtClockwork wants to merge 21 commits intomasterfrom
jlarabie/2.0-private-tables

Conversation

@JasonAtClockwork
Copy link
Contributor

@JasonAtClockwork JasonAtClockwork commented Feb 3, 2026

Description of Changes

Updated the codegen table iteration functions to take in a parameter to include private and defaulted it false in all locations for the supported languages.

  • Updated the util.rs functions for iterating tables to check for a boolean flag include_private and defaulted it false in all used places
  • Updated the CLI to return a list of all private tables not included (added a TODO to check the --include-private opt):
Optimising module with wasm-opt...
Build finished successfully.
Skipping private tables during codegen: secret_note, secret_order, secret_person.
Generate finished successfully.

API and ABI breaking changes

Technically API breaking as the private tables will no longer be available. (GitHub labels are not working at the moment)

Expected complexity level and risk

1 - Simple change the testing took longer

Testing

Turns out when you remove private tables you invalidate most of the module_bindings across the system!

  • Rust test SDK for all languages
  • C# SDK tests
  • C# dotnet tests
  • Updated and checked snap files
  • Updated Blackholio (Unreal + Unity) module_bindings and tested
  • Ran Unreal SDK tests

joshua-spacetime and others added 8 commits January 29, 2026 11:16
…ext (#4101)

# Description of Changes

Makes the `sender` field on `ViewContext` private and exposes a
`sender()` method. Does the same for `ReducerContext` and
`ProcedureContext`.

The purpose of this change: So that the host can determine if/when a
view invokes or reads the `sender`.

Currently, because `sender` is a field, the host assumes that it is
always read. This means views must be materialized per client, even if
the view doesn't actually depend on `sender`, resulting in data
duplication.

The initial solution for this problem was `AnonymousViewContext` which
doesn't have a `sender` field. The better solution is to make `sender` a
method so that it can call into the host and record when it's actually
invoked.

Note, this patch only updates the module API, so the current
implementation does not change. `ViewContext` views are still duplicated
across clients. Changing this requires a new host syscall and for
`sender()` to invoke that syscall. This however is backwards compatible
and can be done anytime after the module APIs for the other languages
(C#, TypeScript, C++) are updated.

Also note that `ReducerContext` and `ProcedureContext` were updated
purely for consistency. There are currently no plans to track reads of
`sender` in these contexts.

# API and ABI breaking changes

Breaks the rust module api.

# Expected complexity level and risk

1

# Testing

N/A
# Description of Changes

Make `Sender` a method on [Reducer|View|Procedure|Tx]Context in C#.
Equivalent changes to
#4101.

# API and ABI breaking changes

API breaking

# Expected complexity level and risk

1

# Testing

Pure refactor, no additional testing.
# Description of Changes

Switch Wasm module to `__describe_module_v10__` from
`__describe_module__` to expose `RawModuleDefV10`

# API and ABI breaking changes
2.0 breaking change.

# Expected complexity level and risk
1

# Testing
Exising smoketests should be enough.
@JasonAtClockwork JasonAtClockwork self-assigned this Feb 3, 2026
@JasonAtClockwork JasonAtClockwork marked this pull request as ready for review February 3, 2026 23:18
@JasonAtClockwork JasonAtClockwork changed the base branch from 2.0-breaking-changes to master February 6, 2026 19:42
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.

This PR has somehow wound up including some changes that I don't think should be here. That said, all of the changes do look correct, both the ones related to the PR's goal and the other ones. Not required, but for extra credit: I would consider it better Rust style to use an enum, maybe named CodegenVisibility, as the argument to iter_tables, iter_table_names_and_types, &c., with variants IncludePrivate and OnlyPublic, so the call sites look like iter_tables(module, CodegenVisibility::OnlyPublic, rather than using a boolean and having the less-explicit iter_tables(module, false).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants