-
Notifications
You must be signed in to change notification settings - Fork 121
When packwerk was created, the simple definition - all files in a directory with a package.yml files and all subdirs belong to that package - did not take into account nested packages.
The implementation ended up treating nested packages as just different packages, with no relationship to the parent package whatsoever.
This means that if you declare code in a parent package to be in its own, nested package, it's at the same time removed from the parent package.
I find that this leads to a conflict between two goals:
- Colocating and encapsulating small cohesive units of code
- Enforcing strong boundaries where they are most impactful, which is between large parts of the codebase
Imagine this setup (Rails folder structure omitted for brevity):
billing
- package.yml
- billing_primitives.rb
- buyers
- bill_buyers.rb
- sellers
- bill_sellers.rb
Now if we decide that we want ownership and encapsulation for billing/buyers. We drop in a package.yml:
billing
- package.yml
- billing_primitives.rb
- buyers
- package.yml
- bill_buyers.rb
- sellers
- bill_sellers.rb
billing/buyers/bill_buyers.rb is now no longer part of the billing package, for all intents and purposes. It can have arbitrary dependencies on packages outside of billing that may be opposite of what its parent package has. This problem is especially apparent if the parent package expresses an architectural layer.
Aside: Gusto implemented an architecture checker that partially solves this problem for layers. But it doesn't follow packwerk conventions that package structure is reflected in the directory structure (layers are not packages, and thus don't have to be directories), and adds a different kind of violations that are called architecture but really are just dependency violations. In doing so, it increases complexity of the overall setup.
As a reminder, this is what an application's architecture may look like (not intended to depict an ideal situation):
Example application architecture diagram with horizontal (layers) and vertical separation
I want to propose two changes that could help resolve this tension in a backwards compatible way. Note that these changes are independent of each other; we can do either without the other, both, or none.
One proposal concerns the way we express dependencies, the other concerns the scope in which dependencies are enforced.
Wildcard Dependency Specs
TL;DR:
# merchandising/package.yml
enforce_dependencies: true
dependencies:
- billing/*
Depend on a package and all its nested packages. This would mean that a consumer is not impacted by restructuring in what it depends on.
There's a slight problem with this one: How do we "hide" code? Maybe that means we shouldn't do this. Maybe it just means that existing privacy checkers need some adjustment.
Aggregated Dependency Enforcement
This is the real heart of this proposal.
TL;DR:
# billing/buyers/package.yml
enforce_dependencies: parent
This would mean that for dependency enforcement purposes, files in this package are still part of the parent package. It means that if billing/buyers/bill_buyers.rb references a constant in platform/auth.rb, billing/package.yml needs to specify a dependency on platform. Essentially, it means that dependency enforcement is delegated to the parent package.
When dependency enforcement is delegated to the parent, the package doing the delegation may not specify its own dependencies.
I'm not set on the name parent and open for suggestions. "nested"? "inherited"? "merge_with_parent"?
Notes
I'm a bit jealous of more expressive architecture enforcement tools like Python's import-linter, but we have to remember that Ruby is not Python; it doesn't have a built-in mechanism equivalent to Python's module imports, so we're starting at a different level.
If we wanted similar expressiveness as import-linter, I think we'd have to redesign packwerk from the bottom up. This proposal, in contrast, is about incremental improvements on top of what we have in place, and what we have existing momentum on.
All reactions
Replies: 7 comments 9 replies
At gusto we have discussed this as the problem of "not all violations are created equal" - i.e., we care about certain (cross-domain) violations more than others: (within-domain) violations.
I also find it interesting to ask... what if we succeed in cleaning up billing? Let's imaging we do and make it its own app. Then packwerk/pks would work as is!
With this framing in mind, here are two ideas we have bounced around:
-
Nested
packwerk.ymlfiles in one project. This would create a new packwerk scope for subdirectories: i.e.,billing/packwerk.ymlAn outside scope wouldn't analyze packages inside of a nested scope and treat all of them as the root-folder-package. The nice thing about his implementation is that they would survive code extractions. -
Different names for package.yml. I.e,
billing/billing_package.yml. We'd add a--scopeparameter to packwerk. When passed in, packwerk will only analyze package.ymls within that scope (the prefix of the package.yml file).
There is a bunch of subtleties going on with both and they have slightly different capabilities but try to solve the same problem.
Aside: wildcard dependency specifications (and more generally wildcards in all packwerk and packwerk-extensions list configs) solve are nice in this context and generally in large applications. I feel like adding those is a no-regrets move...
All reactions
-
👍 1
Interesting ideas. I like the first one, but explicit scopes through additional parameters seems a bit too complex? Not sure if it's more complex than my proposal though. 🤔
Can you expand a bit on the nested scopes idea?
All reactions
I like the idea of Aggregated Dependency Enforcement. It feels pragmatic. I imagine some people will want to see explicit dependencies of subpackages and this approach sounds like it would remain backwards compatible.
I'm not set on the name parent and open for suggestions. "nested"? "inherited"? "merge_with_parent"?
Does this overload "enforce_dependencies"? The recent #384 merge makes me think there's an alternative where a new top-level key like "inherit_dependencies" becomes relevant.
It has the downside of only being situationally relevant (when in a nested structure) but I think the distinct representation could be better than introducing more values to "enforce_dependencies".
All reactions
I was thinking an overloading of enforce_dependencies would be good because these are incompatible - you wouldn't be able to have both inherit_dependencies: true and enforce_dependencies: false, for example.
All reactions
@exterm fair. I appreciate the already narrow surface area of keys package.yml supports natively so I was on the fence and it makes sense that true|strict would both apply to nested packages.
All reactions
It may be convenient to have nested packages inherit parent dependencies by default, because it would be easy to forget to specify that explicitly each time for nested packs. And allow nested packages to have dependencies of their own.
All reactions
that's also an interesting idea, since it would preclude fine-grain dependencies that create cycles against coarse-grain dependencies (one of my pet peeves).
All reactions
Counter argument: #392 (reply in thread)
All reactions
Thanks for starting this discussion!
I've always thought of packwerk as basically doing this:
- providing some basic building blocks to declare and analyze package graphs
- provide a sort of "DSL" for constraining that package graph
It makes total sense to me to introduce more sophisticated ways to constrain package graphs that reflect how folks think about design in the real world.
I'm open to us experimenting with different interfaces for doing this. I think that the options you presented @exterm are sharp tools – they give users to express the concepts of nested packages without explicitly saying that it's a nested package per se. This provides a lot more flexibility to the end user, maybe at the expense of simplicity. I do think aggregated/inherited dependency configuration is closer to explicitly calling something a sub-package.
I also really like @astyagun 's idea of treating nested packs as first-class citizens. So if a user nests package B under package A, then without any other configuration, a couple things are true:
- the package is constrained to only use the dependencies of the parent
- clients can only access the parent pack – not the sub-packs (which would be considered implementation details of the parent pack)
Then maybe nested packs can override or configure that default behavior as necessary... I'm not sure what use cases folks had in mind though.
All reactions
clients can only access the parent pack – not the sub-packs (which would be considered implementation details of the parent pack)
I'm not sure about that one. It seems that it would be a kind of privacy mechanism, and would force facades in the parent package.
It's also not how most package systems out there work AFAICT (e.g. you can import nested packages just fine in python).
All reactions
-
👍 2
Yep I wasn't sure about it either. More important is the first class concept of nested packs and reasonable defaults for that.
All reactions
-
👍 1
As a counterargument to these proposals, I've heard feedback that Packwerk does dependency management better than many other tools because there isn't any magic / transitivity. The list of dependencies you see are exactly (statically) what a package needs, and code can be extracted with only this list in mind. At the cost of duplication, we have a good amount of simplicity.
I'm not 100% against these proposed features, but I think it might compromise this simplicity. At Shopify, we've seen a few use-cases where we have nested packages that have entirely different dependencies than their parent. Are we implying in this discussion that this is an anti-pattern?
I think enforce_dependencies: parent makes more sense than wildcards. Wildcards introduce a way to potentially depend on unnecessary packages, and a cost to grep/search for packages. I'll discuss it with the team next week and see if there are any other opinions.
All reactions
Thank you for your thoughts on this, Gannon!
The fact that wildcards essentially introduce dynamic dependencies is an interesting aspect, and I can imagine some drawbacks. Can you expand on your concerns, maybe with an example? Fine granularity would still be possible by deciding not to use wildcards. For more context, part of the motivation of this proposal is that I have run into a few situations where the desired granularity of ownership is finer than the desired granularity of dependency management, at least initially.
we've seen a few use-cases where we have nested packages that have entirely different dependencies than their parent.
Intuitively, I'd say that's a pattern to avoid, since it means that the top levels of the folder structure do not accurately represent the structure of the application. However, I may be convinced otherwise by good arguments or examples.
Can you expand on the examples you have in mind? What do you think would be good reasons to set things up in this way?
All reactions
we've seen a few use-cases where we have nested packages that have entirely different dependencies than their parent
This is how I've understood it. Given:
flowchart LR
subgraph A
B
end
A-->C
If parent package A has code, that depends on C, this doesn't mean, that any code in B depends on C. If so, then my previous suggestion for B to automatically inherit dependencies of A would not be correct indeed.
All reactions
It may be easier to think about this if problems with current implementation are visualized. Here's what I can think of:
Absence of a "circular dependency" error
---
title: 1
---
flowchart LR
subgraph A
B
end
B-->C
C-->A
---
title: 2
---
flowchart LR
subgraph A
B
end
A-->C
C-->B
---
title: 3
---
flowchart LR
subgraph A
B
C
end
B-->D
D-->C
All reactions
If these are problems to solve, then another possible solution is to inherit dependencies (outgoing and incoming) from nested packs to parent packs. But try to mark inherited dependencies somehow, so that they affect dependencies checker, but are not drawn on graphs by tools like Graphwerk and visualize_packs.