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.

Sign up for GitHub

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

Jump to bottom

Generate files from another package #224

Closed
jpearll opened this issue Apr 5, 2023 · 1 comment
Closed

Generate files from another package #224

jpearll opened this issue Apr 5, 2023 · 1 comment
Assignees
@mwhittaker

Comments

@jpearll
Copy link

jpearll commented Apr 5, 2023

Hi, I am pretty new to this project but found it very interesting!

I am working on building a hexagonal architecture structure in Go, and one of the thing I am doing is separating the ports (interface) from their corresponding concrete implementations adapters.

I was looking to compose my adapter with the weaver.Implements[T], where T is the port (interface, abstraction) which reside on another package.

There is this limitation now, of requiring the interface in the same package as the concrete implementation (when using the weaver CLI). For me that goes against the coupling and segregation concerns I want to enforce with this type of architecture.

I was wondering if that limitation be removed in the future.

Thanks
@mwhittaker mwhittaker self-assigned this Apr 5, 2023
@mwhittaker
Copy link
Member

Hi @jpearll! Thanks so much for checking out the project. You're totally correct that right now we require a component interface and component implementation to be in the same package. We elaborate on this more in #98. I'll close this issue as a duplicate of #98, if that's okay with you. We'll definitely re-examine the limitation and see if it can be removed. Feel free to re-open if you have any other questions :)

mwhittaker added a commit that referenced this issue Apr 21, 2023
@mwhittaker
Component interface, struct in different packages.
d409404 
> Overview

Currently, we require that a component interface and its accompanying
implementation be in the same package. We've received a number of
requests on GitHub (#98, #224) and on our [Discord][discord] to allow
the interface and implementation to be in different packages. For
example, one user's application generates gRPC server interfaces in one
package, and they want to implement these as components in a different
package.

Here are four options for how to deal with this issue in increasing
order of complexity. This PR implements the second option, but that
doesn't mean I think it's the best option. It was quick to implement,
and it helped me understand all the options. Though, in reality, yes I
do prefer Option 2.

> Option 1: Disallow It

This option is trivial. Don't change anything and continue requiring
component interfaces and implementations to be in the same package.

> Option 2: Allow It (this PR)

Allow component interfaces and implementations to be in separate
packages. This introduces two issues:

1. A developer may forget to link in a component implementation. Before,
   if you called `weaver.Get[T]`, you were sure to link in the package
   containing `T`, and therefore you would link in the corresponding
   implementation of `T`. With this option, `weaver.Get[T]` is not
   sufficient. A developer must also link in the package with the
   implementation of `T`.

2. A developer may link multiple implementations of a component. This is
   the opposite of the previous problem. Previously, we could detect
   duplicate implementations of the same interface at code generation
   time, but with this option, that becomes impossible. A user may
   generate code separately for two packages, each with an
   implementation of the same interface.

This option appears to have the advantage that it allows people to swap
out different implementations of the same interface, which some users
have requested. That's true, but it's not so straightforward. If the two
components have different configs, for example, swapping is not so easy.

The benefit of this option is that it's simple, and if we encourage
people to place their interfaces and implementations in the same
package, they shouldn't run into any problems. Only advanced users, who
opt to split their interfaces and implementations have the additional
burden of remembering to link in one, and only one, implementation.

> Option 3: `weaver.Get[Impl]`

For this option, we change `weaver.Get`. Rather than writing
`weaver.Get[Intf]` for some interface `Intf`, developers write
`weaver.Get[Impl]` for some implementation type `Impl`. This makes it
impossible for a user to forget to link in an implementation, though it
still allows them to link multiple implementations.

Implementing this option is a bit tricky because `weaver.Get` still
needs to return the interface type, even if it receives the
implementation type. I think this is doable though. Here's a rough
sketch: https://go.dev/play/p/2L7R6JibaNN. Note that this doesn't build
on go 1.20, but it does build on go 1.21.

> Option 4: Impls Are Components

This option builds on Option 3. `weaver.Get` takes an implementation
type. However, we take it a step further and make a radical shift in the
way we view components. A component is no longer uniquely identified by
its interface, but rather by its implementation. Even if two
implementations implement the same interface, we view them as distinct
components.

This option avoids forgetting to link a component and avoids double
linking a component. It's also not so different from the way we had
things way back in the day where we automatically generated interfaces
from implementations.

[discord]: https://discord.gg/FzbQ3SM8R5
@mwhittaker mwhittaker mentioned this issue Apr 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mwhittaker mwhittaker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mwhittaker @jpearll