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

feat: Show graph for file:// + optional isolation #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danopia
Copy link
Member

@danopia danopia commented Jan 29, 2022

@izelnakri (context) was looking for a Deno graphviz tool which can be used locally, and noticed that I treat all local files as just one file:// 'module':

graph

So I decided to use a better default of treating each directory as a module: (Note that deps.ts is special cased to prevent false-positive of cycles when subdirectories used the parent's deps.ts)

graph

And also added an --isolate-files CLI flag to make one node per file:

graph

Hoping this proves useful now.

@izelnakri
Copy link
Contributor

izelnakri commented Jan 29, 2022

@danopia this is awesome, thanks for working on it! I particularly liked the different coloring between std, dep, and project files. Also having a list of directories in each dependency addition looks great! However I was thinking of a more general/customizable CLI interface. What do you think of this?

$ deno-graph test/some_module.ts # shows all files in the graph(including files in external dependencies)
$ deno-graph test/some_module.ts group-by=module # external dependency origin summary + file graph for internal files
$ deno-graph test/some_module.ts group-by=origin  # current behavior: group by external dependencies, and a single node for the internal.

This previous suggestion I made seems lacking some consideration as well. Idea I had was for the cli to operate on input files rather than on a project just like any test runner.

It seems there are 4 spheres and 2 toggles. 4 spheres:

  • std,
  • external dependency,
  • input targets
  • input folder (could be ignored and ${CWD}/deps.ts chosen if no target file provided)
  • project folder(could be ignored and . could refer to the input folder reference).

2 toggles:

  • Display the graph by folder summary(single node)
  • Display the graph by file summary(many nodes connected)

Based on these we can group the spheres by toggles and if no flag provided, make it as detailed as possible:

$ deno-graph test/some_module.ts  test/some_module_2.ts # equivalent to --group-std-by=file --group-deps-by=file --group-project-files-by=file
$ deno-graph test/some_module.ts test/some_module_2.ts  --group-std-by=folder --group-deps-by=folder --group-project-files-by=file

So I suggest 3 optional flags and 2 toggles for these flags: file or folder. What do you think?

@danopia
Copy link
Member Author

danopia commented Jan 30, 2022

Thanks for the feedback. I think it's also important - for this project's goal at least - to consider how new functionality would be useful on the web interface as well. I have #3 open to track this, with the example of clicking one box -> a view where that specific package is exploded into its constituent files.

Grouping into file / std / third-party makes some sense, but unfortunately there's a longer list of known registries and also when you are looking at a published module via the web interface, even the main module would be considered a third-party dependency by current logic.

So given these concerns I think bucketing might not be so simple, but I agree that explicit grouping settings would be a useful feature and something worth designing. Currently I only have an 'isolated std' flag which would be easily expressed using your example flags.

Not super related, but I've also considering Graphviz subgraphs for any module which is 'exploded' but haven't had a proper chance to see how applicable the feature would be for me.

@izelnakri
Copy link
Contributor

but unfortunately there's a longer list of known registries and also when you are looking at a published module via the web interface, even the main module would be considered a third-party dependency by current logic.

Yes, coloring logic could be separated as a different feature/flag and could perhaps even accept a optional config file/reference(package.json) for this if needed.

Also subgraphs could be additional nice step in the future/release bump indeed.

Let me know if I could help in anyway, thanks again for working on this!

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

Successfully merging this pull request may close these issues.

2 participants