nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.63k stars 2.36k forks source link

workspace.json is not normalized, introduce a key to specify project.json and normalize workspace.json #8078

Closed DavidGamba closed 2 years ago

DavidGamba commented 2 years ago

Description

As per the documentation here: https://nx.dev/l/n/core-concepts/configuration#projectjson A project can be defined inside the workpace.json in two ways, fully explicitly or by giving a string to the project.json location:

{

  "version": 2,
  "projects": {

// Option 1

    "myapp": {
      "root": "apps/myapp/",
      "sourceRoot": "apps/myapp/src",
      "projectType": "application",
      "targets": {
...
     }

// Option 2

    "myapp-with-project-json": "apps/myapp-with-project-json"

  }
}

This leads to a workspace.json file that can have values of type "string" or an object/struct. This breaks other tools trying to parse the file, and it is particularly problematic in monorepos where you might want to leverage these files with multiple languages.

For example:

$ jq -r '.projects[].root' workspace.json 
apps/myapp-with-project-json
jq: error (at workspace.json:4531): Cannot index string with string "root"

jq doesn't fail when the key is missing, only because the schema is not consistent.

To fix this problem, introduce a way to tell NX about the project.json:

    "myapp-with-project-json": {
      "projectJson": "apps/myapp-with-project-json",
    }

And mark option 2 above as deprecated or discouraged. Eventually remove it. Projects that need the normalized workspace.json will move to the new syntax.

Motivation

Allow tools with strict schemas to make use of workspace.json

Suggested Implementation

Introduce the "projectJson" key.

Alternate Implementations

Reuse the "root" key, though this requires extra logic to figure out if everything is in there or not. We might always want to check if there are project.json overrides at the project root location or something like that.

AgentEnder commented 2 years ago

Hey @DavidGamba!

Generally workspaces pick one of the formats and stick with it, with very few actually choosing to mix and match (though it is supported). For any tool which is distributed outside of a single Nx monorepo, I would strongly recommend using either readWorkspaceConfiguration from @nrwl/workspace, or the Workspaces class from @nrwl/tao/src/shared for reading workspace.json. These will ensure that you receive the info you expect consistently if we make updates in the future to the on-disk config format.

Even for shorter scripts that are in a single repo, if you write them in JavaScript or TypeScript you can take advantage of the functions above, and that would be my recommendation.

We are continuing to tweak things with our config format, to make it a bit more streamlined and let users opt-in to some of the more verbose configuration options when they are needed.

Does this explanation help understand the current direction? Adding these as a property complicates the logic in a few places and would end up w/ workspace.json being much longer (3 lines / project compared to 1 line / project), so I'm not sure that's a path that we would want to take.

DavidGamba commented 2 years ago

Hi @AgentEnder, thanks for getting back to me.

The problem with the approach of having to go through the javascript/typescript libraries is that you alienate other languages in a tool used for monorepos.

As for future updates, the workspace.json file does include a version number so it is just a matter of incrementing that to notify of schema changes.

I don't think verbosity trumps correctness and with the current approach I can't build a schema validator that passes (my monorepo is using the mixed approach). You can also have a json one liner: "myapp": { "projectJson": "path/to/myapp" }

AgentEnder commented 2 years ago

The current format can certainly be validated w/ json schema, as nx-console provides such a schema to vscode. Let me see if I can grab that example.

AgentEnder commented 2 years ago

Here's the schema that we use there:

{
    "title": "JSON schema for Nx workspaces",
    "id": "https://nx.dev",
    "type": "object",
    "properties": {
      "version": {
        "type": "number",
        "enum": [1, 2]
      }
    },
    "allOf": [
      {
        "if": {
          "properties": { "version": { "const": 1 } },
          "required": ["version"]
        },
        "then": {
          "description": "Read more about this workspace file at https://nx.dev/latest/angular/getting-started/configuration",
          "properties": {
            "projects": {
              "type": "object",
              "additionalProperties": {
                "type": "object",
                "properties": {
                  "architect": {
                    "description": "Configures all the targets which define what tasks you can run against the project",
                    "additionalProperties": {
                      "type": "object",
                      "properties": {
                        "builder": {
                          "description": "The function that Nx will invoke when you run this architect",
                          "type": "string"
                        },
                        "options": {
                          "type": "object"
                        },
                        "configurations": {
                          "description": "provides extra sets of values that will be merged into the options map",
                          "additionalProperties": {
                            "type": "object"
                          }
                        }
                      },
                      "allOf": [
                        // ... dynamically generated builders config
                      ]
                    }
                  }
                }
              }
            }
          }
        }
      },
      {
        "if": {
          "properties": { "version": { "const": 2 } },
          "required": ["version"]
        },
        "then": {
          "description": "Read more about this workspace file at https://nx.dev/latest/react/getting-started/configuration",
          "properties": {
           "projects": {
              "type": "object",
              "additionalProperties": {
                "oneOf": [
                  {
                    "type": "string"
                  },
                  {
                   "type": "object",
                   "properties": {
                      "targets": {
                        "description": "Configures all the targets which define what tasks you can run against the project",
                        "additionalProperties": {
                          "type": "object",
                          "properties": {
                            "executor": {
                              "description": "The function that Nx will invoke when you run this target",
                              "type": "string"
                            },
                            "options": {
                              "type": "object"
                            },
                            "configurations": {
                              "description": "provides extra sets of values that will be merged into the options map",
                              "additionalProperties": {
                                "type": "object"
                              }
                            }
                          },
                          "allOf": [
                                // ... dynamically generated executors config
                          ]
                        }
                      }
                    }
                  }
                ]
              }
            }
          }
        }
      }
    ]
  }
AgentEnder commented 2 years ago

As for the use of the libraries alienating other languages, those are just what we provide to make parsing the file easier. You could do so on your own, or write a thin node script wrapper that is essentially just something like

read-workspace-config.ts

console.log(JSON.stringify(require(@nrwl/workspace).readWorkspaceConfiguration()))
DavidGamba commented 2 years ago

Thanks for the responses. TIL that oneOf was part of a valid JSON schema. I still think that it would be great if the schema didn't involve oneOf and tools like jq could just parse it.

There is no way to represent that datastructure in other languages like Go Strucs so unmarshalling requires two steps and a type assertion too.

But I do appreciate you having taken the time to explain the rationale behind the current design.

Feel free to close with issue if you feel this is not where you want to take the tool.

AgentEnder commented 2 years ago

I really would like to re-emphasize using a light node wrapper if you want to read the workspace in another programming language. Its a bit more work than just reading the file, but should lessen the possibilities of your custom tools breaking during an Nx upgrade. If you're using Nx, you already are running node and such so its not too much of an ask imho.

Its ultimately not my decision if we want to pursue changes like this, so I'll talk it over with the team.

I'm going to close this out for now, and keep it in mind to reopen if we go down this path.

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.