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

Remove type safety from Interface #38

Closed
sunli829 opened this issue Apr 29, 2020 · 12 comments
Closed

Remove type safety from Interface #38

sunli829 opened this issue Apr 29, 2020 · 12 comments
Labels
enhancement New feature or request

Comments

@sunli829
Copy link
Collaborator

sunli829 commented Apr 29, 2020

The type safety of the interface causes some functionality to be unavailable. I'll remove the compile-time type safety of the interface and check it when I call Schema::new. It doesn't have any performance loss, but there is no guarantee that a compile-time type safety, I think it is worth to do so.

@sunli829 sunli829 added the enhancement New feature or request label Apr 29, 2020
@sunli829
Copy link
Collaborator Author

The side effect is improved runtime performance.😂

@phated
Copy link
Contributor

phated commented Apr 29, 2020

I'm not very happy with this idea.

@sunli829
Copy link
Collaborator Author

sunli829 commented Apr 29, 2020

If the interface field types do not match, there is now a panic at Schema::new and a reason is given.

The following code didn't compile well before, but it does now.

struct MyObj;

#[Object]
impl MyObj {
    async fn a(&self) -> &str {}
}

#[Interface(field(name = "a", type = "String"))]
struct MyInterface(MyObj);

This actually seems reasonable, it simplifies the definition of the interface.

@phated
Copy link
Contributor

phated commented Apr 29, 2020

Panic is not desirable. I believe there is a solution that involves traits but I'm unable to work on it currently.

@sunli829
Copy link
Collaborator Author

sunli829 commented Apr 29, 2020

Panic is not desirable. I believe there is a solution that involves traits but I'm unable to work on it currently.

@phated
Thanks for your suggestion.😁 I will make a version as soon as possible and see how it works.

@phated
Copy link
Contributor

phated commented Apr 29, 2020

@sunli829 is it possible to use a macro to add additional methods to a trait, including a default implementation? That is the solution I am considering.

@sunli829
Copy link
Collaborator Author

sunli829 commented Apr 29, 2020

@phated I'm actually hoping to solve this problem #29 and #28. I think procedural macros may not be possible.

@phated
Copy link
Contributor

phated commented Apr 29, 2020

I am hoping to find time to understand macros better, but my other open source responsibilities are requiring my time right now.

Tomorrow I am giving a presentation on async-graphql and I wanted to show type safety on interfaces.

@sunli829
Copy link
Collaborator Author

@phated I tried this on a new branch.

In addition, thank you very much for the support of async-graphql. I was even more excited when I found out that you are the main maintainer of glup.js.

@phated
Copy link
Contributor

phated commented Apr 29, 2020

🙇 Thank you! Your work on this project is amazing and I very much appreciate it! I am excited to help out more.

@nicolaiunrein
Copy link
Collaborator

@sunli829 I think it is a great Idea especially when it panics at application start. I never really liked the implementation of Interface. I think it is somehow reversed. Isn’t there a more natural way where Interface behaves more like a trait? So that Objects and SimpleObjects can "implement" it rather than beeing passed into the interface as struct fields? It just feels wrong and doesn’t allow shareing an interface over crate boundaries.

@phated Is your presentation being recorded? I would love to see it.

@sunli829
Copy link
Collaborator Author

@nicolaiunrein The interface definition is not natural, but it is simple enough to use.😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants