cornucopia-rs / cornucopia

Generate type-checked Rust from your PostgreSQL.
Other
759 stars 31 forks source link

Work with both sync and async #180

Closed Virgiel closed 1 year ago

Virgiel commented 1 year ago

fix https://github.com/cornucopia-rs/cornucopia/issues/176

Support generating async and sync code reusing common types.

If only async or sync code is generated we reuse the current module tree :

pub mod types {
    // Common database declared types (Enum & composite)
}
pub mod queries {
    pub mod module {
        // Common queries types (Row, Params & Borrowed)
        // Generated type for synchronous or asynchronous usage (Stmt & Query)
    }
}

If both sync and async code are generated we add new modules:

pub mod types {
    // Common database declared types (Enum & composite)
}
pub mod queries {
    pub mod module {
        // Common queries types (Row, Params & Borrowed)
        // Generated type for synchronous usage (Stmt & Query)
        pub mod sync { /* ... */ }
        // Generated type for asynchronous usage (Stmt & Query)
        pub mod async_ { /* ... */ }
    }
}

The CLI still generates async code by default and can generate async and sync when both flags are provided --sync --async

Even if some inner generated code has changed, this PR should not be a breaking change.

LouisGariepy commented 1 year ago

Should we use r#async or tokio for the async module?

As a user-facing module, my preference would be async_ as it is a fairly standard way to disambiguate keywords. If we go for pure correctness, then r#async would also work.

Another way to disambiguate that is used in rustc is to change a letter, like crate -> krate, so async -> asynk could work, but I personally don't really like that (although I could be convinced).

To me tokio is misleading, it looks like it could be a module that reexports items from tokio.

How should we handle sync/async combination for the CLI?

--sync and --async flags are fine for me. To generate both, you'd do both: --sync --async.

According to crates.io, our async client has twice as many downloads, so I think the default (when none of these flags are given) should still be only async.

LouisGariepy commented 1 year ago

I'll try to make some time and review this (finally! :upside_down_face:). Are you still OK with this PR @Virgiel ?

Virgiel commented 1 year ago

@LouisGariepy Ready to merge !

LouisGariepy commented 1 year ago

Looks good to me!