specta-rs / rspc

A framework for building typesafe web backends in Rust
https://rspc.dev
MIT License
1.17k stars 55 forks source link

Specta silently overwrites types defined with duplicate names #101

Open lostfictions opened 1 year ago

lostfictions commented 1 year ago

I've been using a pattern like this when defining a router:

let router = Router::<Arc<prisma::PrismaClient>>::new()
    .query("coolQuery", |t| {
        #[derive(Type, Deserialize)]
        pub struct Input {
            some_arg: String,
            whatever: Option<u32>
        }

        t(|db, input: Input| async move { /* ... */ })
    )
    .query("niceQuery", |t| {
        #[derive(Type, Deserialize)]
        pub struct Input {
            different_arg: i64,
            cool_thing: String
        }

        t(|db, input: Input| async move { /* ... */ })
    )
    .build()
    .arced();

Rust is totally fine with defining structs in a lexical scope like this -- and since I don't use them anywhere else in my code, it feels ergonomic to colocate them next to the procedure definition.

Specta doesn't mind this either -- but unfortunately, when multiple structs have the same name (as with Input above) it'll silently pick one to emit and ignore the rest. This was pretty confusing to me -- I think maybe it should just panic if it encounters an existing name instead?

Brendonovich commented 1 year ago

A quick fix should be to add #[specta(inline)] to each Input struct. This will stop rspc from generating a dedicated type for each Input struct, instead you can access each procedure's argument types using inferProcedureInput from @rspc/client. As a permanent solution we're considering inlining all input types, no matter if they're defined with #[specta(inline)] or not. This would encourage use of inferProcedureInput which I personally think should be used instead of dedicated types.

lostfictions commented 1 year ago

hmm, #[specta(inline)] on a struct doesn't seem to be working for me:

        .query("posts", |t| {
            #[derive(Type, Deserialize, Debug)]
            #[specta(inline)]
            struct PostsInput {
                #[specta(optional)]
                offset: Option<u32>,
                #[specta(optional)]
                count: Option<u32>,
            }

            t(|db, input: PostsInput| async move {
                // ...
            })
        })

yields:

export type Procedures = {
    queries: 
        { key: "posts", input: PostsInput, result: /* ... */ }
        // ...
    mutations: never,
    subscriptions: never
};

export interface PostsInput { offset?: number, count?: number }

which is the same output as when the attribute isn't present.

I'm using rspc@0.1.2 but it looks like there's been significant work on the main branch since then... maybe something was fixed?

oscartbeaumont commented 1 year ago

It's possible that isn't in the latest release. I am a bit behind on releasing changes because there are some breaking changes on main which I haven't decided what my plan is with them, once I'm happy with that stuff I will do a release. If you want to try main ensure you also upgrade your frontend packages (I publish beta releases to npm every commit). You will probably wanna refer to the examples for the newer syntax when defining the rspc client on the frontend because the transports system has been replaced with a tRPC style links system. Sorry for the inconvenience!

It's also entirely possible rspc is just overriding the inline, I can't remember how it works off the top of my head.

lostfictions commented 1 year ago

No worries! This isn't a huge issue for me since it's easy enough to stick to a more explicit naming convention that avoids clashes for now -- happy to wait for a new release.