Detect circular dependencies [Requires: Cross-file checking]

[miazbikowski]

Acceptance Criteria

• Explore how we could identify problematic circular dependencies
• Depending on the result of the exploration open a PR or present your findings to the team

Notes

• It's possible to get into an infinite loop with blocks and we wouldn't currently catch this before somebody blows up their theme
• It's also possible to have a block render a block and it not be a problem (e.g. early return if a value is set) and we wouldn't want to throw a theme check error in that case

Detect and warn on circular dependencies. Ex: Group block has a static Text block and the Text block itself has a static Group block. This can lead to infinite loop in adding a Group block preset since static blocks are added with their container (block that contains them)

[miazbikowski]

I consulted with @charlespwd on this and explored a straight-forward solution in which you take a file's content_for "block" items and for reach one, reach into that type's file and do the same check for content_for "block" and continue until you've detected that a file you're visiting was visited already.

That solution is potentially O(N^2) - and with it we have concerns:

Using AST or regexes to get the content_for "block"s would probably be the same amount of lift. We technically already have access to the AST of all the files we pass to theme check. The problem is preloading & traversal. If you have a cyclical dependency that spans 5 files, 5 files might report the same error for different entry points. Which means all 5 files would need to do the same work.

We might want something like a "this runs only once check that can report errors for multiple files", but theme-check 2 was specifically built not to do this. So dealing with the implications of that change would be a big lift

We have several questions to ask ourselves about these new types of checks and their behaviour:

what do we do in vscode?

• do we run them all the time?
• do we run them on save only? do we run that in cli only? what about admin theme code editor? how would we report that? when would we report that? where would we report that? LSP diagnostics have a non-optional file URI baked in them, if we have a cyclical dependency that spans multiple files, what do we do? N errors at all the locations?

(and probably more questions we have yet to think of)

[graygilmore]

@charlespwd would you mind adding context on why this is blocked? merci!

[charlespwd]

Hmmmm this feels pre-vfs, pre-preloading of all the files. Not blocked anymore. We could probably do something with the theme graph 🤔?