consider removing/disallowing schema cycles

[rogpeppe]

Currently a small number of the lexicons introduce cycles, a.k.a. circular dependencies.
This makes it awkward to generate code in languages such as Go which don't allow cyclic package dependencies.

One way to do it is to merge all lexicons into a single package, but that's not ideal:
keeping an isomorphism between lexicons and the generated code seems like a nice property.

Here's an itemized list of all the circular dependencies currently (as of commit e9ba4a7).
Each cycle is shown as a separate section. The indented parts show the actual definition-level dependencies.

app.bsky.actor.defs ->
	#viewerState -> #listViewBasic
app.bsky.graph.defs ->
	#listItemView -> #profileView
	#listView -> #profileView
app.bsky.actor.defs

app.bsky.embed.record ->
	#viewRecord -> #view
app.bsky.embed.recordWithMedia ->
	#view -> #view
	#main -> *
app.bsky.embed.record

app.bsky.embed.record ->
	#view -> #generatorView
app.bsky.feed.defs ->
	#postView -> #view
app.bsky.embed.recordWithMedia ->
	#view -> #view
	#main -> *
app.bsky.embed.record

app.bsky.embed.record ->
	#view -> #generatorView
app.bsky.feed.defs ->
	#postView -> #view
app.bsky.embed.record

It's possible to break the cycles with a three line change, although clearly that would break compatibility.

diff --git a/lexicons/app/bsky/actor/defs.json b/lexicons/app/bsky/actor/defs.json
index 3674c224..44c2543c 100644
--- a/lexicons/app/bsky/actor/defs.json
+++ b/lexicons/app/bsky/actor/defs.json
@@ -80,7 +80,6 @@
       "type": "object",
       "properties": {
         "muted": {"type": "boolean"},
-        "mutedByList": {"type": "ref", "ref": "app.bsky.graph.defs#listViewBasic"},
         "blockedBy": {"type": "boolean"},
         "blocking": {"type": "string", "format": "at-uri"},
         "following": {"type": "string", "format": "at-uri"},
diff --git a/lexicons/app/bsky/embed/record.json b/lexicons/app/bsky/embed/record.json
index bf0b21cb..1810fce6 100644
--- a/lexicons/app/bsky/embed/record.json
+++ b/lexicons/app/bsky/embed/record.json
@@ -19,8 +19,7 @@
           "refs": [
             "#viewRecord",
             "#viewNotFound",
-            "#viewBlocked",
-            "app.bsky.feed.defs#generatorView"
+            "#viewBlocked"
           ]
         }
       }
@@ -44,8 +43,7 @@
             "refs": [
               "app.bsky.embed.images#view",
               "app.bsky.embed.external#view",
-              "app.bsky.embed.record#view",
-              "app.bsky.embed.recordWithMedia#view"
+              "app.bsky.embed.record#view"
             ]
           }
         },

[dholms]

hmmm 🤔

I see two different issues here:

First: .defs is a catchall for definitions in a given namespace. There aren't actually circular dependencies between schema defs, but rather between the files.

We could break these circular dependencies at the definition layer, but that seems a bit awkward - we'd be splitting up defs like profileView & profileViewBasic

Second & probably harder to resolve: app.bsky.embed.record & app.bsky.embed.recordWithMedia actually do have a circular dependency as application semantics. A embed.record can contain an embed.recordWithMedia & vice-versa. Breaking those up at the schema layer sounds pretty unintuitive

---

One possible middleground solution would be to group all defs (not records/queries/procedures/subscriptions) in a single file & then have separate code-genned files for each of the higher order lexicons

Thoughts?

[rogpeppe]

We could break these circular dependencies at the definition layer, but that seems a bit awkward - we'd be splitting up defs like profileView & profileViewBasic

One possibility in this specific case might be to fold all the definitions from app.bsky.graph.defs into app.bsky.actor.defs. Then there's no need to split them up: the cycle is happily contained inside a single lexicon file. It seems like it should be possible to keep the old definitions around inside app.bsky.graph.defs as references into app.bsky.actor.defs, hence preserving compatibility. Apart from one wrinkle: the modlist token. Moving a token from one place to another changes its identity AFAICS, so I'm thinking it's not possible to move modlist from graph to actor without breaking compatibility, but if it's left inside graph, then listPurpose will still be referring to the graph definitions so there's still a cycle. But the other way around (move all the definitions into actor from graph) is still possible, because actor does not define any tokens.

Second & probably harder to resolve: app.bsky.embed.record & app.bsky.embed.recordWithMedia actually do have a circular dependency as application semantics. A embed.record can contain an embed.recordWithMedia & vice-versa. Breaking those up at the schema layer sounds pretty unintuitive

This is potentially not hard to resolve AFAICS, by moving app.bsky.embed.recordWithMedia#view inside app.bsky.embed.record (call it mediaView, say). Then as in the example above, the cycle is contained in a single file; recordWithMedia#view can be be changed to be a reference to the new type, thus preserving compatibility.

Here's a patch that demonstrates both possibilities (but it does move the modlist token, so probably moving the definitions from actor to graph is a better approach.

In general, my experience with Go packages tells me that although avoiding cycles is a pain at times, the resulting acyclic dependency patterns are almost always better engineered and easier to understand, so it might well be worth sorting out/clarifying this issue while it's easy to do.

One possible middleground solution would be to group all defs (not records/queries/procedures/subscriptions) in a single file & then have separate code-genned files for each of the higher order lexicons

This doesn't seem like it would scale very well to me when there are an arbitrary number of namespaces around, because we might want to generate code for different namespaces in different, separately generated, modules, which precludes having a single unified view of all definitions in one place.