As a library developer, I want to have candid_path rethought so that consumers of my library do not need to depend on ic_cdk directly

[lastmjs]

Is your feature request related to a problem? Please describe.

I am developing Sudograph, a procedural macro that is meant to be used within a Rust canister. Sudograph generates a GraphQL database, and reexports ic_cdk and other crates from cdk-rs. Right now, I have it designed so that the consumer of Sudograph only needs to depend on sudograph in their Cargo.toml for their canister. But as I've gotten into enabling cross-canister GraphQL resolvers, the CandidType attribute macro has caused me problems. Because of the way candid_path is designed, see here: https://github.com/dfinity/candid/blob/master/rust/candid_derive/src/lib.rs#L36 I have to force the consumers of Sudograph to depend directly on ic_cdk, when otherwise they would not have to. I think the root of the problem is hardcoding this path ::ic_cdk::export::candid, especially the prefix ::.

Describe the solution you'd like

Consider changing the hard-coded candid_path to ic_cdk::export::candid, or allow the developer to set the crate location manually in another macro. For example, serde had a very similar problem that was solved with the introduction of this #[serde(crate="self::serde")], allowing the developer to specify the location of the serde crate (or something like that, I'm still trying to wrap my head around Rust's packages, crates, and modules, but the serde solution works).

Describe alternatives you've considered

I could always force the consumer of Sudograph to depend directly on ic_cdk, but it would be an extra inconvenience to them. Part of Sudograph's value proposition is being extremely simple, a sort of one-stop shop for developing on the IC using GraphQL. I have been messing with modules and crates to try and get this to work without needing changes to the candid libraries, but I'm not sure it's possible, and the fix on your end probably isn't too complicated.

Additional context

I can give more context on the serde issue if desired, I would need to dig up the issue. Please understand that this is about the ergonomics and ease of use of libraries like Sudograph, right now the CandidType attribute macro is somewhat tightly coupled to a specific project structure and could be made more composable.

[chenyan-dfinity]

Let me try to better understand the problem here. The reason :: prefix doesn't work is because sudograph exports its own version of ic_cdk? Note that even if we fix this, the user would still need to directly depend on serde, because the derive code for Deserialize is hardcoded to serde.

I was basically following serde's solution. Last time I checked, I think they have a :: prefix. But that was a few years ago. Hardcoding the path is never a good idea, so I am open to any improvement suggestions or pull requests.

[lastmjs]

The reason :: doesn't work isn't because Sudograph is a proc macro that users invoke in their own crate. The best developer experience I can think of is to not require my users to depend on anything but Sudograph. So, the :: prefix used in CandidType looks for ic_cdk in the user's crate. I have reexported ic_cdk from sudograph, which has thus far allowed me to not require the user to depend on ic_cdk directly from their crate. Serde isn't a problem either, I have fixed that by using [serde(crate="self::serde")].

Basically I need to be able to tell CandidType where ic_cdk is, having it hard-coded is causing issues. Let me find the serde issue that really explains this

[lastmjs]

And here is the issue: serde-rs/serde#1465

I believe the problem I had with serde is very similar to the problem I am now having with CandidType. You might need to use the serde solution within this project as well?

[lastmjs]

I imagine a solution might look something like this: [candid(crate="self::sudograph::ic_cdk")]

[chenyan-dfinity]

Thanks for the link. I was looking at the same issue two years ago, and there was no solution at that time. #[candid(crate="self")] looks like a good solution.

We have some higher priority tasks, so it will take some time before I can handle this issue. I would be happy to review a pull request from you if you want things to go faster. Another immediate workaround is to clone candid in your repo, and then you can do whatever change you want.

[lastmjs]

Glad we're seeing the same thing now :) I might clone the repo and do a pull request, seems the fastest way to solve my problem, though I also have some priorities and can wait. We'll see, thanks for engaging on this issue

…

On Sat, Jun 19, 2021, 12:40 PM Yan Chen ***@***.***> wrote: Thanks for the link. I was looking at the same issue two years ago, and there was no solution at that time. #[candid(crate="self")] looks like a good solution. We have some higher priority tasks, so it will take some time before I can handle this issue. I would be happy to review a pull request from you if you want things to go faster. Another immediate workaround is to clone candid in your repo, and then you can do whatever change you want. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#248 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVNFD66BRDFMEXG6Z2SV6TTTTJCJANCNFSM466PO7PA> .

[lastmjs]

I cloned the repo into Sudograph and have confirmed that changing the path manually works. Now I'll experiment a bit with #[candid(crate="self")], hopefully it won't be too hard. I'll pull request if I get it.

[lastmjs]

Pull request: #249

I left some comments as I'm not sure on what conventions you have or what is best practice for unwrapping etc. I also went with #[candid_path("path_to_candid")] instead of #[candid(crate="self")] because it was much easier to implement (I'm not very familiar with all of the syn/quote parsing functionality) and still seems idiomatic enough.