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

Option to stop inheritance of cache keys from dependencies #237

Closed
aomarks opened this issue May 16, 2022 · 0 comments · Fixed by #238
Closed

Option to stop inheritance of cache keys from dependencies #237

aomarks opened this issue May 16, 2022 · 0 comments · Fixed by #238
Assignees

Comments

@aomarks
Copy link
Member

aomarks commented May 16, 2022

Currently the cache key for a script (which affects whether it can be skipped or restored-from-cache) automatically includes the transitive cache keys of all of its dependencies. That means if A depends on B, then if an input to B changes, then both B and A will re-run -- regardless of whether B produces any new or relevant output.

This is a safe and good default because it doesn't require you to specify your input files, which means you can start seeing benefits sooner, and makes it harder to create an invalid config at first.

The downside is that you sometimes get unnecessary builds, because sometimes a dependency runs, but doesn't produce new output -- or doesn't produce new output that you actually care about. For example, a change to a .ts file might only cause changes to d.ts files, which Rollup can typically ignore. Or a change to a tsconfig.json type-checking rule might not produce any different output at all.

We should add a way to annotate that a particular dependency edge doesn't inherit the cache key.

@aomarks aomarks self-assigned this May 16, 2022
@aomarks aomarks moved this to 🤔 In Review in Lit Project Board May 16, 2022
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"]
    }
  }
}
```
Repository owner moved this from 🤔 In Review to ✅ Done in Lit Project Board Nov 7, 2022
aomarks added a commit that referenced this issue Nov 9, 2022
First it was `soft`, then it was `triggersRerun`, now it's `cascade` (note we haven't released yet, so none of these changes were breaking).

This name came to me last night, and I kind of like it. I think `soft` is too vague and non-technical, but `triggersRerun` is just a mouthful (I've been typing it out a bunch, and it's been getting on my nerves). `cascade` feels like it sort of gives an impression of what the thing does, and I think will be memorable and understandable once you've read the docs.

Speaking of, I also rewrote the docs in a way that I hope is more clear.

Also adds some missing docs about `dependencies` and `service` to the configuration reference table.

Addon to #237
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

Successfully merging a pull request may close this issue.

1 participant