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

Concise way to depend on scripts in child and dependency workspaces #23

Open
aomarks opened this issue Mar 22, 2022 · 13 comments
Open

Concise way to depend on scripts in child and dependency workspaces #23

aomarks opened this issue Mar 22, 2022 · 13 comments

Comments

@aomarks
Copy link
Member

aomarks commented Mar 22, 2022

[1] Run <script> in all of my workspaces (parent → child).

With Wireit, you can already just do npm run build -ws to run a given script in all workspaces, which is the standard npm workspaces approach. However it's not fully optimal, because npm doesn't parallelize. So we will also support a syntax like this:

{
  "name": "root",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "packages": "workspaces"
        }
      ]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

Which is equivalent to:

{
  "name": "root",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": {
        [
          "./packages/foo:build",
          "./packages/bar:build"
        ]
      }
    }
  }
}

[2] Run <script> in all of my dependencies (child → siblings).

Related, it is often useful to run some script in all of the current packages dependencies (where those dependencies are workspaces contained by the same workspace root).

{
  "name": "foo",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "packages": "dependencies"
        }
      ]
    }
  },
  "dependencies": {
    "bar",
    "baz"
  }
}

Which is equivalent to:

{
  "name": "foo",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        "../bar:build",
        "../baz:build"
      ]
    }
  }
}
@aomarks aomarks self-assigned this Mar 22, 2022
@aomarks aomarks moved this to 🔥 Front Burner in Lit Project Board Apr 1, 2022
@aomarks aomarks self-assigned this Apr 12, 2022
@aomarks aomarks removed their assignment Apr 25, 2022
@aomarks aomarks moved this from 🔥 Front Burner to 🧊 Icebox in Lit Project Board May 6, 2022
@aomarks aomarks changed the title Support $WORKSPACES syntax Concise way to depend on a script in all workspaces May 23, 2022
@appsforartists
Copy link
Member

appsforartists commented May 26, 2022

I'm faking this in the interim by adding this to the root package.json

{
  "private": true,
  "workspaces": [
    "packages/*"
  ],
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        "./packages/core:build",
        "./packages/preact:build"
      ]
    }
  },

where core and preact are the folder names of my local packages. Now I can, e.g. yarn run build watch and have all my packages transitively watched without running into serial blocking issues.

@black7375
Copy link

black7375 commented Jun 8, 2022

IMO, I hope it can be set like turborepo's dependsOn.
If packages.json has workspaces or file pnpm-workspaces.yaml exists, it can be recognized as a workspace.

{
  "scripts": {
    "build": "wireit",
    "lint": "wireit",
    "test": "wiret",
    "test:all": "wiret"  // Root only
  },
  "wireit": {
    "build": {
      "dependencies": ["^build"]
    },
    "lint": {
     },
    "test": {
     "dependencies": ["build"]
    },
    "test:all": {
      "dependencies": ["lint", "test"]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

@rictic
Copy link
Member

rictic commented Jun 8, 2022

What would "^build" in a dependency mean?

@black7375
Copy link

It's a topological dependency.
The explanation is well described in turborepo link.

@aomarks
Copy link
Member Author

aomarks commented Jun 8, 2022

Just to clarify, there are two related operations related to this space (originally this issue was just discussing [1], but I think it's helpful to discuss both together).

[1] Run <script> in all of my workspaces (parent → child).

In Turborepo, I believe [1] just happens automatically, so if you run turbo run build from your root, it will run build in all workspaces.

With Wireit, you can already just do npm run build -ws to achieve this, which is the standard npm workspaces approach. However it's not fully optimal, because npm doesn't parallelize. So we will also support a syntax like this:

{
  "name": "root",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "packages": "workspaces"
        }
      ]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

Which is equivalent to:

{
  "name": "root",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": {
        [
          "./packages/foo:build",
          "./packages/bar:build"
        ]
      }
    }
  }
}

[2] Run <script> in all of my dependencies (child → siblings).

This is what the ^build syntax in Turborepo does.

In Wireit I think we should support it with a similar explicit syntax to [1]:

{
  "name": "foo",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "packages": "dependencies"
        }
      ]
    }
  },
  "dependencies": {
    "bar",
    "baz"
  }
}

Which is equivalent to:

{
  "name": "foo",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        "../bar:build",
        "../baz:build"
      ]
    }
  }
}

@appsforartists
Copy link
Member

Mild bikeshedding, but dependencies can have both modules in your control (my-other-package, perhaps populated with a symlink by yarn link my-other-package) and ones downloaded from npm (e.g. tslib). Curious to see how you handle that.

Would "packages": "dependencies" only follow symlinks?

@aomarks
Copy link
Member Author

aomarks commented Jun 8, 2022

Mild bikeshedding, but dependencies can have both modules in your control (my-other-package, perhaps populated with a symlink by yarn link my-other-package) and ones downloaded from npm (e.g. tslib). Curious to see how you handle that.

Would "packages": "dependencies" only follow symlinks?

Great question! I think there is indeed an important distinction between sibling workspace dependencies, and npm link'd packages. I think we only want to include the former.

I think the right solution is probably to find your workspace root, grab the set of all workspaces, and then filter them by the dependencies of the starting package. Or in other words, it's the intersection of your dependencies and your sibling workspaces.

@black7375
Copy link

black7375 commented Jun 9, 2022

My first suggestion was that I would like to be implicit syntax at workspace level.

{
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "workspaces": true
        }
      ]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

to

{
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

The explicit syntax is easy to implement and may have advantages in the performance, but it becomes too complicated.
For example, if you convert the first proposal to an explicit syntax,

{
  "scripts": {
    "build": "wireit",
    "lint": "wireit",
    "test": "wiret",
    "test:all": "wiret"
  },
  "wireit": {
    "build": {
      "dependencies": ["^build"]
    },
    "lint": {
     },
    "test": {
     "dependencies": ["build"]
    },
    "test:all": {
      "dependencies": ["lint", "test"]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}

It is inconvenient because it is too long and needs to set various files.
I think it is good to dependencies build at the top level in terms of cohesion.

{
  "scripts": {
    "build": "wireit",
    "lint": "wireit",
    "test": "wiret",
    "test:all": "wiret"
  },
  "wireit": {
    "build":  {
      "dependencies": [
        {
          "script": "build",
          "packages": "workspaces"
        }
      ]
    },
    "lint": {
      "dependencies": [
        {
          "script": "lint",
          "packages": "workspaces"
        }
      ]
     },
    "test": {
      "dependencies": [
        "build",
        {
          "script": "test",
          "packages": "workspaces"
        }
      ]
    },
    "test:all": {
      "dependencies": ["lint", "test"]
    }
  },
  "workspaces": [
    "packages/foo",
    "packages/bar"
  ]
}
{
  "name": "foo",
  "scripts": {
    "build": "wireit"
  },
  "wireit": {
    "build": {
      "dependencies": [
        {
          "script": "build",
          "packages": "dependencies"
        }
      ]
    }
  },
  "dependencies": {
    "bar",
    "baz"
  }
}

Not only turborepo but also yarn manage it at the workspace level.

@aomarks aomarks moved this from 🧊 Icebox to 📋 Triaged in Lit Project Board Jun 10, 2022
@aomarks aomarks changed the title Concise way to depend on a script in all workspaces Concise way to depend on scripts in child and sibling workspaces Jun 10, 2022
@aomarks aomarks changed the title Concise way to depend on scripts in child and sibling workspaces Concise way to depend on scripts in child and dependency workspaces Jun 10, 2022
aomarks added a commit to lit/lit that referenced this issue Jun 29, 2022
Replaces all `lerna run` scripts with `wireit` scripts.

### Benefits

- Increased parallelism during build/test.

- We'll now only be using Lerna for bootstrapping, so we can replace it with npm workspaces in a followup PR.

- It's now easier to figure out which packages failed, because the log statements will say e.g. `[packages/lit-html:build] Failed ...` instead of `[build] Failed`.

### Enumerated packages

This does include some long enumerations of packages. Once we have google/wireit#23, we could replace this with something like:

```json
  "wireit": {
    "test": {
      "dependencies": [
        {
          "script": "test",
          "packages": "workspaces"
        }
      ]
    }
  }
```

However, we currently exclude some packages, e.g. we don't run `lit-html`/`lit-element` package tests, because they are subsumed by the `tests` package. Previously we excluded them with Lerna `--ignore` flags. Now they are just not in the list.

Maybe once we have the above Wireit feature, we could add `test:ci` scripts, which would not be present in the case of packages whose tests we don't want to run in CI.

### Also

- Fixes an incorrect virtualizer output path.

- Fixes an output issue where the lit-html `build:version-stability-test` config was emitting `polyfill-support.js`, which collided with the same file emitted by `build:rollup`.

- Bumps the wtr tests finished timeout. Running both the SSR and main browser tests in parallel makes tests take longer. I guess because of resource starvation.

- Added some additional dependencies to the new `testing` package.

Fixes #2724
Part of #3093
aomarks added a commit that referenced this issue Nov 7, 2022
### Background

Currently, if script A depends on script B, then script B's fingerprint is automatically included in script A's fingerprint. That means if an input file for script B changes, which causes B to re-run, then script A will re-run too — _regardless of whether script B actually emitted different output._

This is a good and safe default because it means you aren't required to specify the input files for every script when those input files are generated by a dependency; Wireit assumes that any time script B runs, script A could be affected. However, it comes with the trade-off that scripts will sometimes re-run even when none of their input files changed.

### This PR

This PR adds a `"triggersRerun": false` setting that can be annotated on dependencies. This prevents the fingerprint from being inherited, so running B won't *necessarily* cause A to run as well.

This can be used for more optimal builds via fewer re-runs, but requires that input files are always fully specified.

This will also be very useful for [service mode](#33), because a `triggersRerun:false` dependency won't require a restart across watch iterations. (Restarts will happen only if the fingerprint is different between iterations). For example, the script that builds the service itself can be a regular dependency (causing restarts), but the script that builds the *assets* served by the script can be a `triggersRerun:false` dependency (not causing restarts).

Note in order to have a place to put this annotation, dependencies can now be objects. They can still be plain strings as well, but must be objects to receive annotations. (This also opens the door for other annotations on dependency edges we may want in the future. E.g. #23 is probably better expressed as a `"workspaces": true` annotation instead of a magic `$WORKSPACES:` prefix as previously planned).

Fixes #237

#### Example

In this example, `bundle` has a `triggersRerune:false` dependency on `build`. Importantly, it also includes `lib/**/*.js` in its `files` array, which are the specific outputs from `tsc` that `rollup` consumes. Including these input files wasn't necessary before, but with `triggersRerun:false` it is now critical.

The advantage of this configuration is that if `tsc` re-runs but doesn't produce different `.js` files (for example, if it only produced different `.d.ts` files), then `rollup` won't need to re-run. We've also excluded the `lib/test` directory, because we know test files aren't included in our bundles.

```json
{
  "scripts": {
    "build": "wireit",
    "bundle": "wireit"
  },
  "wireit": {
    "build": {
      "command": "tsc",
      "files": ["src/**/*.ts", "tsconfig.json"],
      "output": ["lib/**"]
    },
    "bundle": {
      "command": "rollup -c",
      "dependencies": [
        {
          "script": "build",
          "triggersRerun": false
        }
      ],
      "files": ["rollup.config.json", "lib/**/*.js", "!lib/test"],
      "output": ["dist/bundle.js"]
    }
  }
}
```
@cefn
Copy link

cefn commented Nov 29, 2022

Maybe I misunderstood the scope of this feature, but I'd be expecting globs and optionally lists in the workspaces and dependencies fields, as I would value e.g. having different tasks for my apps vs packages, or maybe different text matches in package names. For example my current need is something like this...

{
  "wireit":{
    "test":{
      "dependencies":[
        "test:*"
      ],
    },
    "test:apps":{
       "dependencies":[
         {
           "script:"test",
           "workspaces":"apps/*"
         }
       ]
    },
    "test:packages":{
       "dependencies":[
         "test:packages:store",
         "test:packages:queue",
       ]
    },
    "test:packages:store":{
       "dependencies":[
         {
           "script:"test",
           "workspaces":"packages/store*"
         }
       ]
    },
    "test:packages:queue":{
       "dependencies":[
         {
           "script:"test",
           "workspaces":"packages/queue*"
         }
       ]
    },
  }
}

Currently I'm having to automate the composition of an array like this to give me parallelism which is pretty unpleasant (and fragile) compared to being able to glob it based on an assumed-consistent structure. I haven't bothered to create aliases either (e.g. apps vs packages) as it's too complicated to keep all the lists complete. Typically the apps have MUCH more complex tests and longer runtimes, so I'll often focus on package (unit) tests for rapid TDD feedback, even though the apps DO depend on the packages. Aliasing apps vs packages would mean I can suppress apps in the short-term, but run them in the end. Given the current expressivity I tend to just live with serial execution, or manually watch package-specific test scripts. I'd rather this was able to be done in an aliasing layer.

{
  "wireit": {
    "test": {
      "dependencies": [
        "./apps/counter-react-ts:test",
        "./packages/queue:test",
        "./packages/store:test",
        "./packages/store-edit:test",
        "./packages/store-follow:test",
        "./packages/store-react:test"
      ]
    }
  }
}

@alyahmedaly
Copy link

alyahmedaly commented Dec 19, 2022

is it possible to have glob on dependencies
for example

{
  "wireit": {
    "test": {
      "dependencies": [
        "./packages/*:test"
      ]
    }
  }
}

I think this will make it less magic and easier to understand

@boeckMt
Copy link

boeckMt commented Apr 4, 2023

is it possible to have glob on dependencies
for example
...

Maybe this could be complicated, if not all packages from packages/* are dependencies for the script.
E.g. package lib2 depends on lib3, lib4 and lib5

"workspaces": [
    "packages\\libs\\lib1",
    "packages\\libs\\lib2",
    "packages\\libs\\lib3",
    "packages\\libs\\lib4",
    "packages\\libs\\lib5",
   ...
  ]
{
  "name": "lib2",
  "dependencies": {
    "lib3": "*",
    "lib4": "*",
    "lib5": "*"
  },
  ...
  "wireit": {
    "test": {
      "dependencies": [
        "./packages/libs/*:build"
      ]
    }
  }
}

then we would have something like

"wireit": {
    "test": {
      "dependencies": [
        "./packages/libs/*(lib3|lib4|lib5):build"

        or

        "./packages/libs/!(lib1|lib2):build"
      ]
    }
  }

If all dependencies should be listed implicitly, I like the suggestion of #491

#491 Maybe we could introduce a "command" like wireit detect-dependencies...

@deebloodd
Copy link

is it possible to have glob on dependencies for example

{
  "wireit": {
    "test": {
      "dependencies": [
        "./packages/*:test"
      ]
    }
  }
}

I think this will make it less magic and easier to understand

I think this would be really nice. I was just looking through the docs to see if you could do this. I was just looking to see if this is something I could do quickly. It avoids a lot of API design and sticks with the original syntax.

@anthonyjpratti
Copy link

@aomarks I'd be interested in implementing this! Any advice or concerns before make an attempt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants