Make exports: :all also export subdomains
#68
Conversation
| # If the export's `owner_boundary` exports all modules, include sub-modules | ||
| %{exports: [{export_module, []}]} -> | ||
| String.starts_with?(to_string(export), to_string(export_module)) |
There was a problem hiding this comment.
From what I understand, this will also export private sub-boundary modules. I find this a bit weird, because we can end up with a scenario that some modules are private to other modules in the common parent boundary, but they can be used by the modules outside of that parent boundary.
So I think we should only include public sub-boundary modules, as mentioned in the last comment in #59.
Bubbling up should also work, for example:
- if
Foo.Bar.Bazis a sub-sub-boundary - and
Foo.BarexportsFoo.Bar.Baz - then exporting all from
Fooshould also include exports fromFoo.Bar.Baz
There was a problem hiding this comment.
Even with the updated exports: [Module] test, this logic still seems to work, I think because this function is only looking at the current export and its immediate parent boundary?
I tried writing a few different tests to see if they'd fail but couldn't find any issues with the exported vs private sub-boundaries.
| """ | ||
| defmodule #{module1} do | ||
| use Boundary, exports: :all | ||
|
|
||
| defmodule Schemas.Foo do def fun(), do: :ok end | ||
| defmodule Schemas.Bar do def fun(), do: :ok end | ||
|
|
||
| defmodule SubModule do | ||
| use Boundary, exports: :all |
There was a problem hiding this comment.
Instead of this I propose exports: [Module], and then we can check that Module.Another (perhaps renamed to Module.Private) is not exported. We also need to expand this test to check if bubbling-up works as expected. This can all be done within the same test.
There was a problem hiding this comment.
Updated the test to use exports: [Module]!
| @@ -1147,13 +1147,27 @@ defmodule Mix.Tasks.Compile.BoundaryTest do | |||
| module1 = unique_module_name() | |||
| module2 = unique_module_name() | |||
|
|
|||
| module_test "exporting all modules", | |||
| module_test "exporting all modules and sub-modules", | |||
There was a problem hiding this comment.
| module_test "exporting all modules and sub-modules", | |
| module_test "exporting all modules and exports of all nested boundaries", |
|
I did some local experiments, and this is still not working properly with respect to sub-sub-boundaries. But it doesn't seem to be the problem of this PR, because I'm able to reproduce the issue on the master too. So I'll just merge this, and then try to resolve it separately. Thanks for the contribution! |
Should resolve #59, though there might be cases I've missed.