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

Nx executes targets out of order when overriding target in project.json #26929

Open
1 of 4 tasks
JacobLey opened this issue Jul 14, 2024 · 6 comments
Open
1 of 4 tasks
Assignees
Labels
scope: core core nx functionality type: bug

Comments

@JacobLey
Copy link
Contributor

Current Behavior

When defining a task graph that has tasks defined in both nx.json and project.json, it is expected that the dependsOn is inherited by the nx.json implemention if not otherwise overridden by project.json.

In fact, this is consistent with the task graph displayed by nx graph.

However, when overriding targets in project.json, the dependency order is not always respected. This conflicts with the graph which claims it still is.

Create two "meta-targets" build and test. These are simply nx:noop executors that point to a series of real targets for user convenicence.

Building is bound to a single target build-impl.
Testing is bound to two targets test-impl and report-impl. These both depend on build being complete.

For simplicity, these default implementations are a 1 second wait, then logging their name.

report-impl is overwritten in project.json to just immediately log the name.

Inspect the task graph nx graph appears to confirm that a test command would:

  1. Run build
  2. Run test
  3. Generate report

Running nx run foo:test actually results in the following logs:

  1. REPORT

  2. BUILD

  3. TEST

The report step is executed immediately, despite the dependency graph claiming otherwise.

Expected Behavior

Running nx run foo:test should result in the following logs:

  1. BUILD

  2. TEST

  3. REPORT

GitHub Repo

https://github.com/JacobLey/issue-recreator/tree/nx-task-dependency-order

Steps to Reproduce

  1. Set up package as described above, or in example repo's README
  2. pnpm i
  • Install necessary packages
  1. Run nx graph
  • Confirm that the report-impl task has both build-imp and test-impl as dependencies
  1. nx run foo:test
  • Inspect the console output shows REPORT before BUILD (and TEST), meaning that it did not properly block on a dependency.

Nx Report

Node   : 22.4.1
OS     : linux-arm64
pnpm   : 9.5.0

nx (global)  : 19.4.3
nx           : 19.4.3
@nrwl/tao    : 19.4.3

Failure Logs

No failure logs, just unexpected execution order.

This _can_ easily induce unrelated failure logs, like in the example above trying to execute tests before the build even starts.

Package Manager Version

pnpm 9.5.0

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

I've tried reducing the example case and don't always see it, but definitely experiencing it 10-fold in my live repo.

Attached is image of nx graph. While the dependency explanation above may seem a bit unclear, hopefully it is fairly obvious here that a fairly linear execution of tasks is expected here. I agree with the output provided by nx graph, it is the actual execution of run that is out of order.
Screenshot 2024-07-13 at 9 54 49 PM

@FrozenPandaz FrozenPandaz added the scope: core core nx functionality label Jul 16, 2024
JacobLey added a commit to JacobLey/leyman that referenced this issue Jul 23, 2024
@jonathanmorley
Copy link
Contributor

This appears to have been introduced in 19.1.1

@jonathanmorley
Copy link
Contributor

I suspect its caused by this #26033

@xiongemi
Copy link
Collaborator

it is actually expected behaviour that dependsOn in project.json overriding the one in nx.json. For the repo, in project.json, for report-impl, since there is no dependsOn specified, it will take that value. So it would consider report-impl does not have any dependencies, hence it runs first.
You can copy the dependsOn value from nx.json to project.json and it will run after test-impl.

@JacobLey
Copy link
Contributor Author

dependsOn in project.json overriding the one in nx.json

Sure, if I specify dependsOn I expect it to override. That is generally consistent with the rest of Nx behavior.

since there is no dependsOn specified, it will take that value

Do you have any documentation to support that? I have not seen anything on those lines, and in fact explicitly see the opposite behavior when testing. Omission of a field is exactly what should trigger inheritance, otherwise how is it ever possible to inherit the default?

I extended my example repo with trivial (nx:noop-backed) tasks a + b. b depends on a, but that is only declared in the nx.json, and project.json re-declares the tasks but omits the dependsOn.

nx.json

{
    "$schema": "./node_modules/nx/schemas/nx-schema.json",
    "cli": { "packageManager": "pnpm" },
    "targetDefaults": {
        "a": {
            "executor": "nx:noop"
        },
        "b": {
            "executor": "nx:noop",
            "dependsOn": ["a"]
        }
    }
}

project.json

{
  "$schema": "../../node_modules/nx/schemas/project-schema.json",
  "name": "foo",
  "targets": {

    "a": {
      "executor": "nx:noop"
    },
    "b": {
      "executor": "nx:noop"
    }
  }
}

Result of nx graph (dependency is maintained):
Screenshot 2024-09-17 at 11 11 09 AM

@xiongemi
Copy link
Collaborator

i think the task graph is wrong

@JacobLey
Copy link
Contributor Author

JacobLey commented Oct 2, 2024

If literally everything (including nothing) overrides the default dependencies, what is the point of having them?

I still believe the expected behavior is to inherit dependsOn (when omitted), but if it truly is expected to be overridden every time, then I would change my request on this issue to:

  1. Remove dependsOn from nx.json's targetDefaults
  2. Fix task graph to no longer inherit the value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: core core nx functionality type: bug
Projects
None yet
Development

No branches or pull requests

4 participants