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

Support for more than one interface name #187

Closed
tengstrand opened this issue Mar 10, 2022 · 15 comments
Closed

Support for more than one interface name #187

tengstrand opened this issue Mar 10, 2022 · 15 comments
Assignees
Labels
improvement Not a bug but an improvement to overall user experience

Comments

@tengstrand
Copy link
Collaborator

tengstrand commented Mar 10, 2022

The interface name to use is specified in workspace.edn in the key :interface-ns where the value is stored as a string.
When people want to share code between Clojure and ClojureScript, they cannot use interface as interface name, because it's a reserved word in ClojureScript. A solution to the problem is to allow more than one name for the interface by also allowing the namespaces to be stored as a vector, e.g.:
:interface-ns ["ifc" "interface"].

@tengstrand tengstrand added the improvement Not a bug but an improvement to overall user experience label Mar 10, 2022
@tengstrand tengstrand self-assigned this Mar 10, 2022
@PavlosMelissinos
Copy link

PavlosMelissinos commented Mar 10, 2022

I'm not 100% sure about allowing a vector. Might lead to abuse, e.g. people specifying :interface-ns ["component-a" "component-b"] and then complaining because names clash (I can see that being an issue in larger polylith repos).

As a polylith user, I would rather see a solution that works per component but I understand if that's too much effort. Could be a different key in workspace.edn too:
e.g.

:interface-nses {"component-a" "iface"
                 "component-b" "interface"}

@tengstrand
Copy link
Collaborator Author

But in this case we could specify :interface-ns ["interface" "iface"] and it will be valid in any component, both component-a and component-b in your example (and any other component).

@PavlosMelissinos
Copy link

PavlosMelissinos commented Mar 23, 2022

it will be valid in any component

Exactly. If you add a new item to the interface-ns vector it will also apply to existing bricks. What if one of the bricks already has an implementing namespace with iface or ifc in its name? Won't polylith assume it's an interface after the change (and complain)?

@imrekoszo
Copy link
Contributor

imrekoszo commented Mar 29, 2022

Just a suggestion:

:interface-ns ["interface" "ifc" ;; these apply to all components
               {"iface" #{"component-a" "component-b"} ;; these only apply to selected components
                "port"  #{"component-c"}}]

@tengstrand
Copy link
Collaborator Author

I kind of like the idea that an interface always starts with the string "interface" because then it's easy to find the interface from an IDE. An idea I have is to have a flag in workspace.edn that is set to false by default, but if you set it to true, then the tool will treat all namespaces starting with the "interface" as interfaces.

Valid names could e.g. be interface, interface-cljc or similar.

What do you think @PavlosMelissinos @imrekoszo ?

@imrekoszo
Copy link
Contributor

@tengstrand I'm usually pro-flexibility but I understand if you think my suggestion is overkill.

My main problem with interface is its length but I haven't found a shorter one I like so far. I don't think it's a tragedy if there aren't any shorter possibilities (like in your latest suggestion) but it would be nice in case someone has a short phrase they like.

However, this is your project and I'd suggest you don't add something you dislike.

@seancorfield
Copy link
Contributor

My vote would be for making :interface-ns a vector of strings and letting people decide on which name(s) to use. It doesn't need to be more complicated than that. People will (obviously) need to choose namespace segments that don't conflict with existing namespaces😄but that's true of any convention in any system.

Like @imrekoszo I don't much care for interface either but I also haven't really found a shorter one I like (I'd lean toward api but it's a loaded term and doesn't really have the same connotations as interface anyway).

@PavlosMelissinos
Copy link

PavlosMelissinos commented Jun 19, 2022

A vector is the simplest solution indeed and I don't think any of the alternatives (including my suggestion) are elegant enough to justify the complexity right now.

@tengstrand I'm still a bit skeptical but if a problem comes up it would still be possible to extend your original suggestion with something like what @imrekoszo proposed.

@furkan3ayraktar
Copy link
Collaborator

One of the benefits of Polylith is that it establishes a common language for project teams. When team members discuss about the project, they are going to talk about interfaces and it is valuable to reflect that within the codebase by using the word interface. I feel like in this case adding flexibility reduces one of the benefits of Polylith and makes it hard to communicate. Think about a team member reading the Polylith documentation and sees interfaces everywhere but in their codebase they used 10 different namespace names for interface that do not include the word interface at all.

Since the interface word creates issues in ClojureScript projects, I would suggest we pick another one (like ifc) and allow that one as well. This way, Polylith would allow both interface and ifc by default and we do not need to list a vector of interface names.

Another idea could be to allow users to only define one alternative interface name (something like :alternative-interface-ns), so that the purpose of it is very clear; we have a situation that it is impossible to use the main interface name so that we use the alternative one.

@tengstrand
Copy link
Collaborator Author

tengstrand commented Aug 3, 2022

Hi guys!

I suggest that we keep the key :interface-ns as today, but internally we also accept interface and ifc. If we just start allowing these two, then we could potentially break existing workspaces.

In the documentation we could recommend people to use one of these two, but that would be their choice in the end. The value they put in the :interface-ns key will be used when creating new components.

So if we for example start using interface as interface name, and then at some point change to ifc (or something else) then both new and old interfaces will still be recognised.

We could continue to use interface as the default when creating new workspaces.

It could even be possible to keep interface as the main interface in a component, but put .cljc files under ifc. But I'm not sure if that is a good ide!

Internally we could keep :interface-ns as a string, which would be used when creating components, and introduce e.g. :interface-nss which would be the set of #{:interface-ns "interface" "ifc"} and used when detecting interfaces.

@seancorfield
Copy link
Contributor

I think @tengstrand latest suggestion is a good compromise: support interface and ifc internally but allow users to specify what to use when creating new components (and therefore allow them a single, alternative if they don't like either of the built-in ones).

@tengstrand
Copy link
Collaborator Author

I have implemented this in the issue-187 branch, so please have a look (latest sha is right now b256b1a).

This was referenced May 18, 2023
tengstrand added a commit that referenced this issue May 18, 2023
Always allow 'ifc' and 'interface' as interface names. Issue #187.
@tengstrand
Copy link
Collaborator Author

I have merged this to master. Please check it out @PavlosMelissinos.

@PavlosMelissinos
Copy link

Nice! Looks good to me @tengstrand 🙂

Do you plan to update the docs as well to mention ifc and custom interface names?

@tengstrand
Copy link
Collaborator Author

tengstrand commented May 18, 2023

Cool! You mean the Gitbook docs?
The Gitbook docs will be updated in the next release of 0.2.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Not a bug but an improvement to overall user experience
Projects
None yet
Development

No branches or pull requests

5 participants