Proposal: Private modules (general discussion)

A minor thing, but naming is hard. You would not say “define module private”, more likely you would say “define private module” and when you talk about these modules you would not say “modules private” but “private modules”, we also have “defp” already in place, so how about:

defpmodule

?

1 Like

Given it is defmacrop and not defpmacro, I think it should be defmodulep, even if for consistency. Also, having the p at the end makes it more visible than in the middle of the word.

8 Likes

Fair enough, I don’t think I’ve ever used defmacrop and didn’t know it existed. It should have been the other way around for easy remembering I think, i.e. code that reads like English comes in a bit more natural than arbitrary reversed. Smalltalk did that a lot, Objective-C as well and I always liked it. but anyway, thanks for clarification and I can very much live with that.

Why not the other way around?

declare the module private with an attribute (@private should be enough) and then to use it in your modules add a requirep keyword.
If you don’t requirep compilation fails.

The intent is clear and there cannot be confusion about using a private implementation that could break over time.

Eventually an optional @visible_to attribute could be used to restrict the “requirability” of the module.

2 Likes

Hi everyone,

Here are two concerns about proposals B/C/D. Since the require+alias are necessary, it may make the code too verbose. For example, imagine you have a Phoenix context with this:

alias MyApp.Blog.{Post, Like, Comment}

If we make those modules private, we need to break them individually as:

require MyApp.Blog.Post, as: Post
require MyApp.Blog.Like, as: Like
require MyApp.Blog.Comment, as: Comment

Which is considerably more verbose.

The other concern is that invoking require (or use or import) on a module defined in the same project, the current module will stop compiling until the required module becomes available and then it resumes compilation. This implies two things:

  1. It is not possible to have circular dependencies between private modules. Although circular dependencies are arguably a consequence of poor design, it still is a difference in behaviour;

  2. And there is a chance that if you require too many modules, compilation may slow down, as we need to wait for the require modules to finish compilation;

Those issues can all happen today but they are not very common because we don’t rely on require and similar between modules in the same project that often. However, adding private modules may be enough to make those issues more common and apparent.

This is not enough to block proposals B/C/D but it adds extra concerns to the mentioned proposals.

Note that one possible solution to this problem is to introduce requirep or aliasp. But we are concerned that adding yet another modified (besides alias, import, require, and use) will add more confusion and complexity than help.

7 Likes

I feel A, C, D are a little inconsistent to the current “private” behaviours, ie: defmacrop, defp. The current behaviours of defmacrop and defp are such that “they are only accessible in the module that they are defined in”. But the message that A, C, D send is: “it is discouraged to use this, but you can still use it if you want”.

B is the most consistent, perhaps it is the least useful? But I’d argue we’d have the same benefits if we made defp also “discouraged but allowed”.

Would it be worthwhile to explore a path where private functions are accessible with a special syntax? If that could be done, one can write a private module by making all its functions private…

You are correct. Technically I think it’s it’s semantics, not syntax, but does getting the word wrong really invalidate the point that you’re adding more overhead to a very central part of the language - defining a new module - which will also not be backwards compatible, and that such a choice has negatives inherent to it that should come into judgement on whether it’s a good idea?

I’m not suggesting it’s not worth it to do so, only that it should be a consideration when comparing solutions.

I now see the distinction you’re trying to make (though I’m not sure it’s obvious from your initial talk about being able to access to allow debugging of live systems), but is it a distinction captured in the behavior of the code? Will those other than module authors be prevented from accessing? If not, do you think it’s a distinction that will survive in the wild?

Disagreed with @mischov and @brightball here.

  • In the case of inexperienced programmers: they simply look for the quickest way to get a job done without regarding much of anything else. So they definitely do not have a good reason for invoking private modules.

  • In the case of experienced programmers: they will have zero problems being explicit about their intention of using a private module – which is the whole point of this proposal.

In both cases, if the library doesn’t leave you with any choice but to use a private module then (a) you should try and communicate with the author to fix that or (b) will make a conscious decision to use a frozen version of the library since you rely on its private API.

I don’t see how implementing this proposal will change anything at all in legacy code. Moving forward, it allows for explicitness which I think we all around here agree is a good thing.

I prefer C.

D is too weak. For example, when a function is private, Elixir won’t allow me to just use it, and warn me. I think C is more in line with this. And having the :"Elixirp.(.*)" fallback seems to be as an OK tradeoff because of real world needs. The p in Elixirp makes it super easy to navigate through all the hacks.

I’d prefer having A’s @visible_to attribute, instead of passing it in the head. It seems it would be too common to have multiline defmodulep heads, because of reasonably fine grained visibility lists. And since when we’re defining module stuff, this usually goes through module attributes, @visible_to feels more idiomatic to me.

If someone doesn’t specify @visible_to, I guess we could go with the base namespace as a convention? This.Example.PrivateModule => default to [This]. I think this is nice when the user doesn’t care about using it inside the package, just doesn’t want to expose these modules to external code (in other namespaces).

Would be nice if mix docs would get an option to put in or not private modules (maybe default to false?).

1 Like

It is because you should never have :"Elixirp.Foo.Bar" in your compiled code. It is very easy to spot those transgressions, it looks ugly, and it can be automated by tools like Credo. The concession exists for authors debugging and exploring the code, which means they are accessing it only in temporary sessions, such as inside IEx, and never as part of their compiled code.

1 Like

I vote for C.

Because it doesn’t break the decisions made by the platform (repl, live debug, etc), and still make very clear to the user that it is a private API, and if wants to use it anyway it by its own risk by doing workarounds.

To the notation, I prefer the declaration to be explicitly on the head of defmodulep because it is required to be visible to something otherwise there is no point in declaring. But IMHO should be noted that defmodule is a macro that accepts arguments, and not a keyword in notation of other languages, so newcomers doesn’t get confused.

I’m ok with using require instead of alias, because you require that module to be in the same “scope”, and alias has the impression that the module is already in the same scope.

Thanks for that proposal :heart:

3 Likes

I vote for C

I agree it’s a problem when libraries and apps use private functions; doing nothing may be ok for a while, but I’d rather not depend on the “There’s no documentation, therefore it’s private” convention. I enjoy the explicitness of FP and Elixir, so I think I’ll enjoy that in documentation as well with guardrails.

2 Likes

Here’s my issue.

In OO programming, private methods and variables make sense to restrict the manner in which the state of an object can change.

In an explicitly functional, call function, get response, no side effects context the value of preventing a method call from a space outside of the developer’s initial intent is minimal.

The case that brought this up was just from a published library with a degree of popularity utilizing something that wasn’t supposed to be utilized. A warning alone would have solved this problem because a) the developer would see it and b) anyone who installed and used the library would see it likely resulting in an issue being created, a PR being created and/or a conversation from the library developer to the team to communicate needs better.

I just don’t see any value that can come from potentially increasing the chances for errors in running code when there’s no a discernible benefit over a warning.

2 Likes

Somebody above said that when pulling your deps for the first time, compiling your project yields a huge amount of warnings – and the warning you talk about will be drowned in them. Don’t get me wrong: I’d mercilessly pursue any and all warnings and fix them but (a) package maintainers aren’t always responsive and (b) I have to prioritize my paid work. All of us have only so much brain and finger cycles per day.

This is only one of its goals. In any and all of the 8 programming languages I worked with, private/protected functions are also used to express the intent of “here be dragons”, namely “this here is an implementation detail, you should not ever use this by yourself, please use my public API functions”.

Additionally, in corporate teams it is also being heavily utilized to limit the possibility of unintended dependencies – f.ex. a colleague from another team scans your internal company repo, finds the perfect function to do their job and before you know it, you have 2 team leads and 10 angry developers at your door because you changed a critical API their apps can’t work without.

When this proposal gets implemented, I’ll use it to enforce boundaries in my code, be it internal corporate big mono-repos, or public open-source libraries.

This also has the added benefit of encouraging discussion of yours / somebody else’s code.

3 Likes

I agree with @imetallica that a private module shouldn’t be accessible at all (or maybe inside of its own file), otherwise it would be strange. Maybe a different name like protected or internal would make more sense.

But speaking about the functionality, I like the option C with some considerations. I believe that is too early to add visible_to, it adds some complexity and issues described before, and for the problem described I believe that having something like require MyApp.Private, as: Private, allow_private: true combined with defmodulep would be enough. Internally, the team should know where to use it, and, externally, you can still get a error that can be bypassed if you know what you’re doing. In this case, private maybe still makes sense, since you’re bypassing it.

I can see that and the perspective makes sense.

At the same time, I’ve been doing this professionally for over 15 years and I’ve never run into an instance where private methods were beneficial in server side code. I keep hearing the justification without seeing the benefits.

The initially described issue seems to be more of a problem solved by dependency management for version compatibility than anything else.

1 Like

I would love to hear how that would solve the problem.

@josevalim although already said that elixir and the compiler doesn’t have the concept of “applications”, I think the way forward would be “internal” modules that are only accessible on the current “application”/“library”, since this would:

  1. allow testing, since tests are inside the “application”.
  2. hide the implementation from outsiders.

Which is precisely what we want to do here. I think this should be reconsidered.

I have been thinking about this approach and I even had a proof of concept and it is really inferior to the alias approach mentioned in B/C/D. For example, imagine this macro in your library:

require SomethingPrivate.Helper, as: Helper

defmacro does_something(arg) do
  quote do
    Helper.compute_x(unquote(arg))
  end
end

If it is per project, Helper is now being used in a third party application due to meta-programming and therefore it will not work. With the require+alias system however, once you alias a private module and you have permission to use, you can pass it around and it will always work. In other words, it works more like an ownership system, which is really neat and transparent.

Perhaps, however, the worst about doing the visibility per project is that if you mess it up, like in the example above, you will only find it out when somebody else uses your project. Probably after a new version is released, and that is quite worse than any of the other proposals mentioned here, which requires you to always consider the visibility.

Finally, you have to remember that IEx is its own application and the application you are working on is lost once you build a release and connect to a remote node, which means that this approach would also make it impossible to debug your private modules in a live system.

So besides the compiler not really knowing about applications, you can see that doing something per project is more error prone than the current proposals in many ways.

4 Likes

I’m not a huge fan of this in general, but I’m resigned that we’re going to see it. My main point against it is the extra complexity it adds to the language that every developer is going to have to learn and deal with as long as Elixir exists. What that buys us is a reduced public perception of package upgrades being brittle. Public perception is definitely important, but at the core this is a value judgement between these two concerns.

Developers depending on undocumented modules is a “people problem.” I’ve rarely seen people problems fixed with technological solutions.

11 Likes