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(graph): display expanded task inputs #19597

Merged
merged 7 commits into from
Oct 16, 2023

Conversation

MaxKless
Copy link
Collaborator

This PR adds a list of inputs per task to the graph UI:
image
image
It also lists dependencies' & external inputs in separate sections:
image

Implementation

We've expanded the nx graph command. graph.ts - getExpandedTaskInputs will use the new HashPlanner to calculate all files that contribute to a project's inputs. Because expanding inputs for all projects is potentially time-consuming, we try to do it lazily.

  • When showing the graph via nx graph, we have a live server started in graph.ts. When queried, this calculcates the expanded inputs for only one task.
  • When showing the graph in Nx Console, dev mode or e2es, we don't have a live server. That means we need to eagerly calculate all inputs and store them, just like with the project & task graph.

This lazy loading sets the task input request apart from the project & task graph requests made in the graph UI. Those are made once when loading a route. By contrast, the individual task inputs are loaded when opening the task tooltip. This required us to break out of the loader pattern somewhat to get the task inputs.

We could adapt this approach to load all task inputs per route, which would probably result in a lot of unused computation (because we don't expect folks to actually click on all the different task inputs). It would, however, be more in line with how we're currently doing things and result in cleaner code.

@vercel
Copy link

vercel bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 7:03am

@MaxKless MaxKless force-pushed the display-expanded-task-inputs branch from 4b8b9f8 to 73471b5 Compare October 13, 2023 14:14
@MaxKless MaxKless requested a review from a team as a code owner October 13, 2023 18:51
@MaxKless MaxKless force-pushed the display-expanded-task-inputs branch from 422457b to 2b3e3ed Compare October 16, 2023 07:03
Comment on lines +857 to +862
function expandInputs(
inputs: string[],
project: ProjectGraphProjectNode,
allWorkspaceFiles: FileData[],
depGraphClientResponse: ProjectGraphClientResponse
): Record<string, string[]> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be replaced by the rust functionality that is going to be built out soon. But for now it's fine to have here. I'll replace this when the new function is ready.

@Cammisuli Cammisuli self-requested a review October 16, 2023 18:19
Copy link
Member

@Cammisuli Cammisuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to have a follow up PR to replace the expandInputs function with a rust one to match the task hasher.

We also should take a look at the actual performance of the task view in the graph because it's quite bloated and slow. But we'll do that in a future PR as well.

@FrozenPandaz FrozenPandaz merged commit c727a22 into master Oct 16, 2023
2 checks passed
@FrozenPandaz FrozenPandaz deleted the display-expanded-task-inputs branch October 16, 2023 20:01
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants