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

Specta silently overwrites types defined with duplicate names #101

Open
lostfictions opened this issue Jan 2, 2023 · 4 comments
Open

Specta silently overwrites types defined with duplicate names #101

lostfictions opened this issue Jan 2, 2023 · 4 comments
Labels
v1.x A change for version 1.x.x
Milestone

Comments

@lostfictions
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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 [email protected] but it looks like there's been significant work on the main branch since then... maybe something was fixed?

@oscartbeaumont
Copy link
Member

oscartbeaumont commented Jan 6, 2023

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
Copy link
Author

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.

@oscartbeaumont oscartbeaumont mentioned this issue Jan 8, 2023
51 tasks
@oscartbeaumont oscartbeaumont added the v1.x A change for version 1.x.x label May 10, 2023
@Brendonovich Brendonovich added this to the 1.0.0 milestone May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.x A change for version 1.x.x
Development

No branches or pull requests

3 participants