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(core): expose the task graph in the executor context #17111

Conversation

leosvelperez
Copy link
Member

@leosvelperez leosvelperez commented May 19, 2023

Current Behavior

The task graph of the current execution is not accessible from executors.

Expected Behavior

The task graph of the current execution is accessible from executors as part of the ExecutorContext. This is helpful for accessing already parsed task dependencies and acting on them.

Related Issue(s)

Fixes #

@leosvelperez leosvelperez self-assigned this May 19, 2023
@vercel
Copy link

vercel bot commented May 19, 2023

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

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2023 7:32am

Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

I've not went over this super in-depth yet, but quick question:

@@ -36,7 +28,8 @@ export function convertNxExecutor(executor: Executor) {
projectsConfigurations,
nxJsonConfiguration,
cwd: process.cwd(),
projectGraph,
projectGraph: null,
Copy link
Member

Choose a reason for hiding this comment

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

Why are we not passing in the project graph anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is in line with another change @vsavkin did on a separate PR (for some reason, we have convertNxExecutor in nx and in devkit). I just aligned the devkit implementation with the change done in the nx one. I asked Victor about that change, and the reasoning is that if you have a builder in an Angular CLI project, there is no meaningful project graph because you aren’t in an Nx workspace and our invariants won’t hold.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, interesting. We had a todo comment about marking the project graph as required in the interface there, we should make sure docs around the compat layer mention that the field will be undefined for Ng CLI workspaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Overall, the docs around Nx and Angular need a good review and more details about the integration and caveats. I have that on my list of things to do. This is not the only thing that's not documented properly.

@leosvelperez leosvelperez force-pushed the core/expose-task-graph-in-executor-context branch from 8d10c2b to 53b5e39 Compare May 19, 2023 15:53
@leosvelperez leosvelperez force-pushed the core/expose-task-graph-in-executor-context branch 2 times, most recently from e5a19d4 to cfbce73 Compare May 22, 2023 09:26
@leosvelperez leosvelperez merged commit 8f6fcf2 into nrwl:master May 24, 2023
@leosvelperez leosvelperez deleted the core/expose-task-graph-in-executor-context branch May 24, 2023 07:50
@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 May 30, 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.

2 participants