Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router v2 #326

Draft
wants to merge 70 commits into
base: main
Choose a base branch
from
Draft

Router v2 #326

wants to merge 70 commits into from

Conversation

oscartbeaumont
Copy link
Member

@oscartbeaumont oscartbeaumont commented Dec 2, 2024

Goal: Introduce the new router, make it supporting by integrations (rspc_axum/rspc_tauri) and allow converting a legacy router into it.

General design:

  • Dropped TMeta generic in favor of improved solution in upcoming middleware system. This has always been a half-baked feature.
  • Procedure names now must be fully unqiue. Previously they were namespaces by the type (query/mutation/subscription) but that is no longer the case. You will recieve an error if you don't comply with this.
  • Using the new specta::Language trait for exporting, opening up the door for easily supporting other langauges. lol goodbye specta::Language you won't be missed.
  • Note that the new router isn't directly usable at the moment. It can be converted fromthe older router but can't mount procedures directly as we are going to introduce a new syntax and middleware system in the near future.
  • Dropped Router::config. You must export types via the new router with the new bindings format.
  • New bindings format - explain it? Must change Bindings to BindingsLegacy to use the existing clients.
  • rspc_axum::endpoint now takes rspc::Router2 (you need it for the types anyway)
  • Drop tracing feature with no directly replacement.
  • rspc_tauri::plugin to rspc_plugin_rspc::init (rspc-tauri to tauri-plugin-rspc (remember to yank old crate), and plugin to init)
  • Dropping HTTP integrations in favor on them being manually implemented by the user.

Breaking changes:

  • Requires new Specta release
  • All procedure names must be unique regardless of type.
  • let (routes, types) = Router2::from(legacy_router).build().unwrap(); (must be used for the runtime and the types)
  • Router::config has been removed. You can use types.export_to.
  • rspc_axum::endpoint(routes). Now takes routes instead of rspc::Router.
  • Drop jsonrpc executor API with no real replacement for now
  • Must rename Procedures to ProceduresLegacy in your TS code.
  • Add location field to Procedure
  • Completly new Tauri integration
  • The user is required to enable the legacy feature to get the old bindings format.

For my future reference:
The HTTP interface has been refactored to run on rspc_core::Procedure which is designed around the queries path not being namespaced by the type this means keeping full backwards compatibility is really hard. Given we are going to change the bindings format with Router2 and upgrade the clients to the new stuff first we may as well just do this as a breaking change. The old system being Box<dyn Layer> and Arc<Router> makes it tough to do conversions too.

Future:

  • Legacy migration sorted out:
    • rspc-legacy crate
    • Drop rspc::legacy in favor of rspc-legacy
    • Drop unstable and nolegacy features from rspc in favor of legacy
    • Rename all the new types so they aren't Router2, etc.
    • example-legacy serve the router via Axum. Figure that out?
  • rspc_devtools - fix it
  • rspc_tracing:
    • Drop `rspc_core::Procedure::with_logger``
    • Add Logger in rspc_core
    • Allow Router::setup to inject a Logger instance for rspc-tracing
    • Logger hook should create span with procedure metadata (ensure that span is tracked for the future somehow)
    • Middleware should only add input/result to existing span.
  • rspc_procedure
    • Split out of rspc_core
    • Allow downcasting (DynOutput::value) a non from_*_stream type?
    • Handle invariants of DynInput::new_value. Option::None will reach havoc.
    • Make serde/related crates an optional depedency (cause Binario doesn't need it)
    • ResolverError::new forces usage of Serde for value. Can we allow dyn Any?
    • Error should have a variant for a custom error by middleware (Eg. Zer). I'm not sure we can make it generate a type but it should be able to serialize any data so a JS package could throw types back onto it later.
    • Should DynOutput have a variant for ProcedureError? We can allow manually grabbing it but it would make the serialization code in userspace much cleaner.
    • Simplify ProcedureStream's constructor code
    • ProcedureStream::from_error is broken with flushing logic. The todo!() in ProcedureStream::poll_inner
    • Make flush work
    • trait impls of ProcedureStream::map related types
    • type_name on DynOutput make it useful or remove it
    • Rename DynInput and DynOutput?
    • Drop all Any bounds cause they are implied
    • Rename - require_manual_stream -> block_streaming, stream -> unblock_streaming, etc.
    • Procedures::new
    • impl Fn() -> TCtx or TCtx: Clone. rspc_invalidation makes this annoying.
    • wait_util function similar to the Cloudflare Workers/Vercel feature. How would a CDN implement it properly?
    • Should we call it State or Data?
    • Finish Debug impls + clippy run
    • Documentation
    • Audit for erased_serde in public API
  • Zer::from_request errors handled as an rspc_core::ProcedureError??
  • rspc - cleanup module structure cause it's a mess
  • SFM's
    • What if a SFM tries to call .invalidate?? We can't queue it again without conflicting keys
    • What if a SFM calls flush?? Cause we are well past that point.
    • stream.resolved() || stream.flushable() is bad
    • Delay executing SFM queries until the batch resolves
    • Deduplicating SFM keys for rspc_invalidation
    • Prevent calling .invalidate after streaming has started. What should happen?
    • rspc_invalidation can we skip serde_json::Value?
    • rspc_invalidate Invalidate::Any
    • rspc_invalidate Invalidate::Many
  • Wire protocol
    • Allow extensions to add extra data
    • Custom streaming response format. Differentiate errors vs values and reference to which query/mutation the result is referring to.
    • Poll the runtime while waiting for the next FormData item in the input.
    • Support WebSocket upgrades too
    • Are we going to keep rspc-axum/rspc-actix-web/rspc-http. Heavily dependant on how out of control the code in userspace ends up as.
    • File will bridge AsyncWrite/AsyncBufWrite to Stream<Item = Vec<u8>> so it can reuse the existing streaming machinery. This will allow interleaving multiple file downloads in one response.
      • In pratice this is how Hyper works so as much as I hate the overhead of buffering it's gonna happen anyway.
      • Yield AsBuffer(Vec<u8>) and then we can know to set the Content-Type in the transport layer correctly.
  • New syntax
    • If TError is fixed maybe we put it on the Router cause it's a bit wierd with BaseProcedure?
    • ResolverError takes two args which should both be self and anyhow::Error is not Clone. I think we can do smart stuff with constructors to solve this.
    • rspc::Error2 is super unsafe! It lets you choose to use #[error(...)] for the repr which is a string not TError.
    • Drop rspc::Error2 in favor of Into?
    • Manual flushing option build on the ProcedureStream flushing stuff
    • ProcedureBuilder::subscription implemented
    • Type exporting (right now they are hardcoded to DataType::Unknown)
    • Support exporting statics #285
    • Improve error handling DX #305 (might require Specta major)
    • ProcedureMeta::state called within a Middleware::setup should panic when accessed as it can't be valid.
    • Correctly determine key in Procedure2::builder.
    • specta_serde integration prototype
    • Jump to definition on routers
    • IntoResponse: ! future compat warning
    • Move Procedure2 into modern::procedure or keep in root?
  • Middleware system
    • Middleware::setup and Extension::setup are overriding which is major cringe alert.
    • Extension map function working. Might need TError generic?
    • Break out into rspc_middleware - ResolverInput, ResolverOutput, Middleware, Extension and related stuff
    • Stop any of the middleware depending on rspc and instead use rspc_middleware
    • Prevent applying rspc-invalidation to mutations and subscriptions. Should we make it typesafe?
  • Handling serializer errors
    • ProcedureStreamMap error handling
    • ProcedureError::Serializer or not? If not clear out commented out stuff.
    • I suppose we hardcode an error response to send if we can't serialize the first error.
    • rspc_tauri should handle first-level serializer errors through rspc instead of throwing into Tauri.
  • Tauri
    • What's the new integration going to look like?
    • Fix @rspc/tauri with legacy system
    • Maybe copy File from rspc_http? Idk if their IPC can handle it but I suppose supporting it anyway would be worthwhile.
  • WithDate
    • trait impls
    • Generalise implementation to any Serialize?
    • Use thread_local to enable the special rspc encoding so the type doesn't mess with users other stuff.
    • Require Accept: text/x-rspc for it to use the special encoding.
  • This is blocked on TypeMap + TypeCollection specta#294 & Language in practice specta#297 & releasing specta-rust.

Release:

  • Reserve all crate names
  • Ensure rspc::Router2 can be implemented in userspace.
  • Ensure ProceduresLegacy works with the existing clients.
  • What the deal with keeping the legacy stuff? Maybe rspc_legacy crate?
  • Migration guide

Unsorted:

  • Upgrade dependencies
  • Improve Procedure's Arcing.
  • Should Procedure2::error being Option<DataType> or not?
  • Ability to downcast instead of Serialize on error. Similar to the regular Ok type.

Remaining questions:

  • rspc_devtools - Should we provide a way to set CORS header. If we leave it on the user the URL being the same is a problem.

This PR will be merged but work will continue as #328, #332 and specta-rs/website#9.

@oscartbeaumont oscartbeaumont marked this pull request as draft December 2, 2024 15:38
@oscartbeaumont
Copy link
Member Author

oscartbeaumont commented Dec 19, 2024

Completed:

  • Router: TryFrom<legacy::Router> (maybe use Try and panic cause we will error in build longer term)
  • Router: Default
  • Router: Debug
  • Router::nest
  • Router: Iterator/IntoIterator
  • Router::merge
  • Basic type exporting
  • New bindings support for nested routers
  • Giving Procedure to multiple integrations (Eg. Tauri and Axum) the Iterator makes this potentially problematic.
  • Export the legacy bindings format from a Router2
  • Make Axum work with Router2. Can we do this without breaking changes and switch endpoint to use it too?
  • Error handling in layer_to_procedure
  • Switch out rspc_axum endpoint backend to the new stuff
  • rspc_tauri support
  • rspc_core metadata for publishing + logo in lib.rs
  • Drop jsonrpc executor API
  • Remove rspc dependency from rspc_axum and rspc_tauri to make future stuff easier.
  • Customise the TypeCollection's header. It should says rspc not Specta!
  • Worskspace Cargo lints
  • Virtual workspace like main-rewrite
  • Typescript to Rust jump to definition by abusing source map (mannnn this is cool!)
  • Cleanup ProcedureStream
  • ProcedureStream::next2 vs ProcedureStream::next
  • Tauri use Channel and expose WindowHandle
  • Migrate all middleware and split to integrations directory?
  • ProcedureStream::from_stream_value
  • Fix invalidation middleware in Axum example
  • Drop ProcedureError::Serialize & generic
  • Handle serialization errors in rspc-tauri (don't just unwrap like we do now)
  • Catch panics in ProcedureStream
  • DynInput<'a, 'de> should really be &'a Input<'de>` but that's hard.
  • Consistency between routes and Procedures
  • Concider changing Into<Procedures<TCtx>> to AsRef so it can be .into()ed multiple times then maybe remove Clone impl on Routes?
  • Error handling for duplicate procedure names (Eg. give Router::build an error type)
  • Can Error::serialize return value instead of taking serializer?
  • Break out rspc part of Axum example
  • Request context so we can track queries requiring invalidation. Gonna use TCtx.
  • ProcedureBuilder needs TBaseInput and TBaseResult
  • Procedure2 needs TInput and TResult derived from TBaseX on the ProcedureBuilder
  • IntoMiddleware needs TBaseX
  • Fix ProcedureBuilder::with
  • Introduce ErasedProcedure.
  • Extension as a clone of Middleware.
  • ErasedProcedure::with which takes Extension
  • Procedure2 needs Into<ErasedProcedure<TCtx>> and Router::procedure needs to take that as it's argument.
  • ProcedureStream::should_flush and optional manual stream flushing mechanism like what Mattrax has.
  • Error::code or Error::status. What should we call it?
  • Sort out tracing calls in crates/*
  • drop axum::Endpoint and move the handler into userspace
  • drop status codes completly -> ProcedureStream::next_status and ResolverError::status
  • drop GET/POST /rspc/{procedure_name} and make it all one endpoint POST /rspc.
    • Request is FormData keyed on procedure name and value as the input.
    • Userspace will decode and spawn onto a runtime.
    • Allow rspc_invalidation to add arbitrary queries into the response.
  • ProcedureStream::resolved
  • ProcedureStream::flushable
  • finish ProcedureStream::map and expose all related types in public API
  • Moving State to rspc_core and exposing it on rspc_core::Procedures.
  • Make rspc-openapi an extension

@oscartbeaumont
Copy link
Member Author

oscartbeaumont commented Dec 22, 2024

The following needs to end up in documentation somewhere.

We will delay running SFM invalidated queries until all items in the batch are complete.
This is to ensure we don't have conflicting keys in the response.
The client is expected to send mutations by themselves (not in a batch) but this can't be enforced by the server so we do the above as a fallback.

For queries and subscriptions calling .invalidate is only allowed if streaming hasn't started (as we delay SFC queries until then).

The response body needs to key items something like:

pub enum Key {
    Response(u32), // Reference to the request
    SFM(String, serde_json::Value), // SFM key (procedure name and input)
}

type ResponseBody = HashMap<Key, Value>;

This way we will never have problems with running a SFM query that was also including in the request batch.
The client would be expected to queue resolving promises so the SFM wins as the canonical value.

@oscartbeaumont oscartbeaumont mentioned this pull request Dec 25, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants