Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect circular dependencies [Requires: Cross-file checking] #559

Open
miazbikowski opened this issue Nov 4, 2024 · 1 comment
Open

Comments

@miazbikowski
Copy link
Contributor

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
Copy link
Contributor Author

miazbikowski commented Dec 5, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant