fix(js): properly identify buildable project

[mandarini]

Current Behavior

Right now, we identify if a project is buildable like this:

function isBuildable(target: string, node: ProjectGraphProjectNode): boolean {
  return (
    node.data.targets &&
    node.data.targets[target] &&
    node.data.targets[target].executor !== ''
  );

The target there is the name of the build target of a parent project. In that function, we try to understand if a project's dependency is buildable. This technique is flawed, since we cannot know for sure the name of the build target, and we cannot assume it's the same as the parent project's build target name.

This results in this: If one project imports another project, both projects' build targets need to have the same name. If projects have different names for the build target, then you get an irrelevant error.

Expected Behavior

isBuildable should not rely on a parent project's build target name, but rather look through the child project's executors, and find if it has an executor which is a build executor.

Retain existing behaviour to avoid breaking changes, and also allow it to work for build executors of other plugins (not @nx/*).

Related Issue(s)

Fixes #17764

[vercel]

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	Jun 27, 2023 8:36am

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1306531. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 1 target

• nx affected -t e2e-macos --parallel=1 --base=836cd34ce70649d9191192ccc7a18bc684f7a2b0 --head=1306531fd7db0c6f03bab0eced98df98331ce0e9

---

Sent with 💌 from NxCloud.

[leosvelperez]

One thing to consider is that while this addresses the issue for projects using that list of executors, it leaves out projects that might be using executors from third-party plugins or even nx:run-commands.

This would be a breaking change for any workspace using such a setup. Maybe that was never intended to be supported and it was meant to be used only with executors from first-party plugins (I don't know), but the implementation allowed using others and folks might be using it.

If we don't want to add such a constraint and make a breaking change now, what about we add this new solution as a fallback to the previous one? The previous solution is quicker and would correctly catch the buildable dependency for some scenarios. If we don't find the target, then we look for known executors.

That would still address the issue while keeping the existing behavior. I would avoid changing the current behavior because we might change this later by using the TaskGraph instead of the ProjectGraph. The executor context now contains the task graph, so we can use it now. The project graph knows nothing about target dependencies, but the task graph does.

[mandarini]

@leosvelperez sure, I'll keep the old way in there, too, as a fallback to the new way! Sounds like a good practice, indeed! Maybe this should somehow be in the docs, though, too? @juristr what do you think? For a later addition, of course, not as part of this PR. Like, if you have dependencies, keep the name of the build target consistent, to make sure third-party plugins work?

[leosvelperez]

Maybe this should somehow be in the docs, though, too? @juristr what do you think? For a later addition, of course, not as part of this PR. Like, if you have dependencies, keep the name of the build target consistent, to make sure third-party plugins work?

It's in some docs https://nx.dev/recipes/other/setup-incremental-builds-angular#build-target-name, but it should probably be generalized here https://nx.dev/more-concepts/incremental-builds. That said, the requirement might change soon, if we get to refactor this to use the task graph. When that happens, the target name will no longer be relevant and I think the only relevant thing will be the outputs which is what is consumed with buildable libraries. So maybe we don't rush updating the docs because they might change soon.

[mandarini]

Closing this, and will incorporate in a new effort to refactor some things. For now, the solution to this issue is one of the two:

1. Both build targets should have the same name

OR

2. In the child project, create a "dummy" build target that depends on the intended build target, like so:

    "new-build": {
      "dependsOn": ["build"],
      "command": "echo dummy build",
      "options": {
        "outputPath": "dist/libs/mylib",
        "tsConfig": "libs/mylib/tsconfig.lib.json"
      }
    },
    "build": {
      "executor": "@nx/js:tsc",
...

[github-actions]

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.