diff --git a/CHANGELOG.md b/CHANGELOG.md index aeba85eba..d3d7c1af9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,52 @@ Versioning](https://semver.org/spec/v2.0.0.html). or when another script that depends on it is ready to run. A service is stopped when all scripts that depend on it have finished, or when Wireit is exited. +- Added `"triggersRerun": false` setting to dependencies. + + By default, the fingerprint of a script includes the fingerprints of its + dependencies. This means a script will re-run whenever one of its dependencies + re-runs, even if the output produced by the dependency didn't actually change. + + Now, if a dependency is annotated with `"triggersRerun": false`, then the + fingerprint of that dependency will no longer be included in the script's own + fingerprint. This means a script won't neccessarily re-run just because a + dependency re-ran — though Wireit will still always run the dependency first + if it is not up-to-date. + + Using `"triggersRerun": false` can result in faster builds thanks to fewer + re-runs, but it is very important to specify all of the input files generated + by the dependency which the script depends on in the `files` array. + + Example: + + ```json + { + "wireit": { + "build": { + "command": "tsc", + "files": ["tsconfig.json", "src/**/*.ts"], + "output": "lib/**", + }, + "bundle": { + "command": "rollup -c", + "files": ["rollup.config.json", "lib/**/*.js", "!lib/test"], + "output": "dist/bundle.js", + "dependencies": { + [ + "script": "build", + "triggersRerun": false + ] + } + } + } + } + ``` + +### Changed + +- Added string length > 0 requirement to the `command`, `dependencies`, `files`, + `output`, and `packageLocks` properties in `schema.json`. + ### Fixed - Fixed memory leak in watch mode. diff --git a/README.md b/README.md index 5eb25a60e..aa294babd 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ - [Dependencies](#dependencies) - [Vanilla scripts](#vanilla-scripts) - [Cross-package dependencies](#cross-package-dependencies) + - [Triggers re-run](#triggers-re-run) - [Parallelism](#parallelism) - [Extra arguments](#extra-arguments) - [Input and output files](#input-and-output-files) @@ -168,6 +169,78 @@ workspaces, as well as in other kinds of monorepos. } ``` +### Triggers re-run + +By default, whenever a dependency runs, the script that depends on it will be +marked stale and need to re-run too — regardless of whether the dependency +actually produced new or relevant output. + +This is a 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. +However, it comes with the tradeoff that scripts will sometimes re-run even when +none of their input files changed. + +To change this behavior and further optimize your build, you can use the +`triggersRerun` setting to tell Wireit that all of the input files needed from +a dependency are declared in the `files` array. Now, Wireit won't assume that a +script was stale just because its dependency ran. + +To enable this setting, create an object for your dependency instead of a plain +string, and set `triggersRerun` to `false`: + +```json +{ + "wireit": { + "A": { + "dependencies": [ + { + "script": "B", + "triggersRerun": false + } + ] + } + } +} +``` + +In the following example, `bundle` has a dependency on `build` with +`triggersRerun: false`. 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 neccessary before, but with +`triggersRerun: false` it is now critical. + +```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"] + } + } +} +``` + +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. + ## Parallelism Wireit will run scripts in parallel whenever it is safe to do so according to @@ -408,12 +481,20 @@ expected to exit by itself, set `"service": true`. "command": "node my-server.js", "service": true, "files": ["server-config.json"], - "dependencies": ["build:server", "build:assets"] + "dependencies": [ + "build:server", + { + "script": "build:assets", + "triggersRerun": false + } + ] } } } ``` +### Service lifetime + If a service is run _directly_ (e.g. `npm run serve`), then it will stay running until the user kills Wireit (e.g. `Ctrl-C`). @@ -421,13 +502,24 @@ If a service is a _dependency_ of one or more other scripts, then it will start up before any depending script runs, and will shut down after all depending scripts finish. +### Service restarts + In watch mode, a service will be restarted whenever one of its input files or -dependencies change. +dependencies change, except for dependencies with +[`triggersRerun: false`](#re-run-on-change). + +Use `triggersRerun: false` when the output of a dependency is read dynamically +for each request handled by the service. For example, the static assets of a web +server can often be annotated with `triggersRerun: false`. + +### Service output Services cannot have `output` files, because there is no way for Wireit to know -when a service has finished writing its output. If you have a service that -produces output, you should define a non-service script that depends on it, and -which exits when the service's output is complete. +when a service has finished writing its output. + +If you have a service that produces output, you should define a _non-service_ +script that depends on it, and which exits when the service's output is +complete. ## Failures and errors diff --git a/schema.json b/schema.json index d0fa29ca7..f6fb62c14 100644 --- a/schema.json +++ b/schema.json @@ -14,33 +14,57 @@ }, "command": { "markdownDescription": "The command to run.\n\nThis is a shell command that will be executed, with all binaries from npm dependencies and devDependencies available.\n\nFor example:\n\n```json\n\"command\": \"tsc\"\n```\n\nFor more info, see https://docs.npmjs.com/cli/v8/using-npm/scripts#environment", - "type": "string" + "type": "string", + "minLength": 1 }, "dependencies": { "markdownDescription": "Other npm scripts that will run before this one.\n\nThese scripts do not have to use wireit.\n\nDependencies can refer to scripts in other npm packages by using a relative path with the syntax `:`. All cross-package dependencies should start with a `\".\"`. Cross-package dependencies work well for npm workspaces, as well as in other kinds of monorepos.\n\nFor example:\n\n```json\n\"dependencies\": [\n \"build\",\n \"./packages/foo:build\"\n]\n```\n\nFor more info, see https://github.com/google/wireit#dependencies", "items": { - "type": "string" + "anyOf": [ + { + "type": "string", + "minLength": 1 + }, + { + "type": "object", + "required": ["script"], + "properties": { + "script": { + "markdownDescription": "The name of the script (see `dependencies`).", + "type": "string", + "minLength": 1 + }, + "triggersRerun": { + "markdownDescription": "When `true` (the default), whenever this dependency runs, this script (the dependent) will be marked stale and need to re-run too, regardless of whether the dependency produced new or relevant output. When `false` Wireit won't assume that the dependent is stale just because the dependency ran. This can reduce unnecessary re-building (or restarting in the case of services) when `files` captures all of the relevant output of the dependency.\n\nFor more info, see https://github.com/google/wireit#re-run-on-change", + "type": "boolean" + } + } + } + ] }, "type": "array" }, "files": { "markdownDescription": "The files that this script depends on.\n\nThese are the files that are watched when run with the `watch` argument. They are also used to determine if a script is stale or if its files and dependencies haven't changed and execution can be skipped.\n\nDon't specify `files` unless the array of files (and `dependencies`) are the only things that this script depends on. For example, a script that fetches data over the internet should not have a files array.\n\nThis should be a list of package-relative paths to files, or glob patterns. See https://github.com/google/wireit#glob-patterns for more info on the format of glob patterns.\n\nFor example:\n\n```json\n\"files\": [\n \"src/**/*.ts\"\n]\n```", "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array" }, "output": { "markdownDescription": "The files that this script writes.\n\nThese are the files that are deleted before the script is executed (set `clean` to customize this behavior), and these are the files that are cached if `files` is specified.\n\nThis should be a list of package-relative paths to files, or glob patterns. See https://github.com/google/wireit#glob-patterns for more info on the format of glob patterns.\n\nFor example:\n\n```json\n\"output\": [\n \"lib/**/*\",\n \"!lib/bundle.js\"\n]\n```", "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array" }, "packageLocks": { "markdownDescription": "By default, Wireit automatically treats package-lock.json files in the package directory, plus all parent directories, as input files. This is useful because installing or upgrading your dependencies can affect the behavior of your scripts, so it's important to re-run them whenever your dependencies change.\n\nIf you are using an alternative package manager instead of npm, then your package lock files might be named something else.\n\nFor more info, see: https://github.com/google/wireit#package-locks", "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array" }, diff --git a/src/analyzer.ts b/src/analyzer.ts index add05c20d..a922b3463 100644 --- a/src/analyzer.ts +++ b/src/analyzer.ts @@ -237,7 +237,7 @@ export class Analyzer { for (const failure of failures) { const supercedes = (failure as Partial) .supercedes; - if (supercedes != null) { + if (supercedes !== undefined) { failures.delete(supercedes); } } @@ -318,7 +318,7 @@ export class Analyzer { if (syntaxInfo === undefined || syntaxInfo.scriptNode === undefined) { let node; let reason; - if (syntaxInfo?.wireitConfigNode?.name != null) { + if (syntaxInfo?.wireitConfigNode?.name !== undefined) { node = syntaxInfo.wireitConfigNode.name; reason = 'wireit-config-but-no-script' as const; } else { @@ -511,7 +511,7 @@ export class Analyzer { scriptInfo.wireitConfigNode && findNodeAtLocation(scriptInfo.wireitConfigNode, ['dependencies']); let encounteredError = false; - if (dependenciesAst == null) { + if (dependenciesAst === undefined) { return {dependencies, encounteredError}; } const result = failUnlessArray(dependenciesAst, packageJson.jsonFile); @@ -528,17 +528,104 @@ export class Analyzer { const uniqueDependencies = new Map(); const children = dependenciesAst.children ?? []; for (let i = 0; i < children.length; i++) { + // A dependency can be either a plain string, or an object with a "script" + // property plus optional extra annotations. const maybeUnresolved = children[i]; - const stringResult = failUnlessNonBlankString( - maybeUnresolved, - packageJson.jsonFile - ); - if (!stringResult.ok) { + let specifierResult; + let triggersRerun = true; // Default; + if (maybeUnresolved.type === 'string') { + specifierResult = failUnlessNonBlankString( + maybeUnresolved, + packageJson.jsonFile + ); + if (!specifierResult.ok) { + encounteredError = true; + placeholder.failures.push(specifierResult.error); + continue; + } + } else if (maybeUnresolved.type === 'object') { + specifierResult = findNodeAtLocation(maybeUnresolved, ['script']); + if (specifierResult === undefined) { + encounteredError = true; + placeholder.failures.push({ + type: 'failure', + reason: 'invalid-config-syntax', + script: {packageDir: pathlib.dirname(packageJson.jsonFile.path)}, + diagnostic: { + severity: 'error', + message: `Dependency object must set a "script" property.`, + location: { + file: packageJson.jsonFile, + range: { + offset: maybeUnresolved.offset, + length: maybeUnresolved.length, + }, + }, + }, + }); + continue; + } + specifierResult = failUnlessNonBlankString( + specifierResult, + packageJson.jsonFile + ); + if (!specifierResult.ok) { + encounteredError = true; + placeholder.failures.push(specifierResult.error); + continue; + } + const triggersRerunResult = findNodeAtLocation(maybeUnresolved, [ + 'triggersRerun', + ]); + if (triggersRerunResult !== undefined) { + if ( + triggersRerunResult.value === true || + triggersRerunResult.value === false + ) { + triggersRerun = triggersRerunResult.value; + } else { + encounteredError = true; + placeholder.failures.push({ + type: 'failure', + reason: 'invalid-config-syntax', + script: {packageDir: pathlib.dirname(packageJson.jsonFile.path)}, + diagnostic: { + severity: 'error', + message: `The "triggersRerun" property must be either true or false.`, + location: { + file: packageJson.jsonFile, + range: { + offset: triggersRerunResult.offset, + length: triggersRerunResult.length, + }, + }, + }, + }); + continue; + } + } + } else { encounteredError = true; - placeholder.failures.push(stringResult.error); + placeholder.failures.push({ + type: 'failure', + reason: 'invalid-config-syntax', + script: {packageDir: pathlib.dirname(packageJson.jsonFile.path)}, + diagnostic: { + severity: 'error', + message: `Expected a string or object, but was ${maybeUnresolved.type}.`, + location: { + file: packageJson.jsonFile, + range: { + offset: maybeUnresolved.offset, + length: maybeUnresolved.length, + }, + }, + }, + }); continue; } - const unresolved = stringResult.value; + + const unresolved = specifierResult.value; const result = this._resolveDependency( unresolved, placeholder, @@ -588,8 +675,9 @@ export class Analyzer { uniqueDependencies.set(uniqueKey, unresolved); const placeHolderInfo = this._getPlaceholder(resolved); dependencies.push({ - astNode: unresolved, + specifier: unresolved, config: placeHolderInfo.placeholder, + triggersRerun, }); this._ongoingWorkPromises.push( (async () => { @@ -670,7 +758,7 @@ export class Analyzer { packageJson: PackageJson, syntaxInfo: ScriptSyntaxInfo ): undefined | ArrayNode { - if (syntaxInfo.wireitConfigNode == null) { + if (syntaxInfo.wireitConfigNode === undefined) { return; } const filesNode = findNodeAtLocation(syntaxInfo.wireitConfigNode, [ @@ -704,7 +792,7 @@ export class Analyzer { syntaxInfo: ScriptSyntaxInfo, command: JsonAstNode | undefined ): undefined | ArrayNode { - if (syntaxInfo.wireitConfigNode == null) { + if (syntaxInfo.wireitConfigNode === undefined) { return; } const outputNode = findNodeAtLocation(syntaxInfo.wireitConfigNode, [ @@ -870,7 +958,7 @@ export class Analyzer { syntaxInfo: ScriptSyntaxInfo, files: undefined | ArrayNode ): void { - if (syntaxInfo.wireitConfigNode == null) { + if (syntaxInfo.wireitConfigNode === undefined) { return; } const packageLocksNode = findNodeAtLocation(syntaxInfo.wireitConfigNode, [ @@ -971,7 +1059,7 @@ export class Analyzer { } const trailArray = [...trail].map((key) => { const placeholderInfo = this._placeholders.get(key); - if (placeholderInfo == null) { + if (placeholderInfo === undefined) { throw new Error( `Internal error: placeholder not found for ${key} during cycle detection` ); @@ -993,7 +1081,9 @@ export class Analyzer { // Use the actual value in the array, because this could refer to // a script in another package. const nextName = - nextNode?.astNode?.value ?? next?.name ?? trailArray[cycleStart].name; + nextNode?.specifier?.value ?? + next?.name ?? + trailArray[cycleStart].name; const message = next === trailArray[cycleStart] ? `${JSON.stringify(current.name)} points back to ${JSON.stringify( @@ -1005,7 +1095,7 @@ export class Analyzer { const culpritNode = // This should always be present - nextNode?.astNode ?? + nextNode?.specifier ?? // But failing that, fall back to the best node we have. current.configAstNode?.name ?? current.scriptAstNode?.name; @@ -1046,7 +1136,7 @@ export class Analyzer { }; return {ok: false, error: this._markAsInvalid(config, failure)}; } - if (config.dependencies != null && config.dependencies.length > 0) { + if (config.dependencies.length > 0) { // Sorting means that if the user re-orders the same set of dependencies, // the trail we take in this walk remains the same, so any cycle error // message we might throw will have the same trail, too. This also helps @@ -1095,7 +1185,7 @@ export class Analyzer { } trail.delete(trailKey); } - if (dependencyStillUnvalidated != null) { + if (dependencyStillUnvalidated !== undefined) { // At least one of our dependencies was unvalidated, likely because it // had a syntax error or was missing necessary information. Therefore // we can't transition to valid either. diff --git a/src/config.ts b/src/config.ts index 0c2dd3d6b..8139145e1 100644 --- a/src/config.ts +++ b/src/config.ts @@ -44,9 +44,12 @@ export interface ScriptReferenceWithCommand extends ScriptReference { extraArgs: string[] | undefined; } -export interface Dependency { +export interface Dependency< + Config extends PotentiallyValidScriptConfig = ScriptConfig +> { config: Config; - astNode: JsonAstNode; + specifier: JsonAstNode; + triggersRerun: boolean; } export type ScriptConfig = @@ -132,7 +135,7 @@ interface BaseScriptConfig extends ScriptReference { * directory + script name, but the {@link Executor} then randomizes the order * during execution. */ - dependencies: Array>; + dependencies: Array; /** * The services that need to be started before we can run. diff --git a/src/execution/base.ts b/src/execution/base.ts index ed0864fcc..47a6fcb26 100644 --- a/src/execution/base.ts +++ b/src/execution/base.ts @@ -10,7 +10,7 @@ import {Deferred} from '../util/deferred.js'; import type {Result} from '../error.js'; import type {Executor} from '../executor.js'; -import type {ScriptConfig, ScriptReference} from '../config.js'; +import type {Dependency, ScriptConfig} from '../config.js'; import type {Logger} from '../logging/logger.js'; import type {Failure} from '../event.js'; @@ -70,7 +70,7 @@ export abstract class BaseExecution { * Execute all of this script's dependencies. */ protected async _executeDependencies(): Promise< - Result, Failure[]> + Result, Failure[]> > { // Randomize the order we execute dependencies to make it less likely for a // user to inadvertently depend on any specific order, which could indicate @@ -82,7 +82,7 @@ export abstract class BaseExecution { return this._executor.getExecution(dependency.config).execute(); }) ); - const results: Array<[ScriptReference, Fingerprint]> = []; + const results: Array<[Dependency, Fingerprint]> = []; const errors = new Set(); for (let i = 0; i < dependencyResults.length; i++) { const result = dependencyResults[i]; @@ -91,7 +91,7 @@ export abstract class BaseExecution { errors.add(error); } } else { - results.push([this._config.dependencies[i].config, result.value]); + results.push([this._config.dependencies[i], result.value]); } } if (errors.size > 0) { diff --git a/src/execution/service.ts b/src/execution/service.ts index 952b8870e..92475c10a 100644 --- a/src/execution/service.ts +++ b/src/execution/service.ts @@ -10,7 +10,7 @@ import {Deferred} from '../util/deferred.js'; import {ScriptChildProcess} from '../script-child-process.js'; import type {ExecutionResult} from './base.js'; -import type {ScriptReference, ServiceScriptConfig} from '../config.js'; +import type {Dependency, ServiceScriptConfig} from '../config.js'; import type {Executor} from '../executor.js'; import type {Logger} from '../logging/logger.js'; import type {Failure} from '../event.js'; @@ -348,7 +348,7 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand + depFingerprints: Array<[Dependency, Fingerprint]> ): void { switch (this._state.id) { case 'executingDeps': { diff --git a/src/fingerprint.ts b/src/fingerprint.ts index dd12e49b8..49151c438 100644 --- a/src/fingerprint.ts +++ b/src/fingerprint.ts @@ -11,8 +11,8 @@ import {scriptReferenceToString} from './config.js'; import type { ScriptConfig, - ScriptReference, ScriptReferenceString, + Dependency, } from './config.js'; /** @@ -111,18 +111,23 @@ export class Fingerprint { */ static async compute( script: ScriptConfig, - dependencyFingerprints: Array<[ScriptReference, Fingerprint]> + dependencyFingerprints: Array<[Dependency, Fingerprint]> ): Promise { let allDependenciesAreFullyTracked = true; const filteredDependencyFingerprints: Array< [ScriptReferenceString, FingerprintData] > = []; for (const [dep, depFingerprint] of dependencyFingerprints) { + if (!dep.triggersRerun) { + // triggersRerun: false means the fingerprint of the dependency isn't + // directly inherited. + continue; + } if (!depFingerprint.data.fullyTracked) { allDependenciesAreFullyTracked = false; } filteredDependencyFingerprints.push([ - scriptReferenceToString(dep), + scriptReferenceToString(dep.config), depFingerprint.data, ]); } diff --git a/src/ide.ts b/src/ide.ts index f03fee068..eb89fcf5f 100644 --- a/src/ide.ts +++ b/src/ide.ts @@ -110,10 +110,10 @@ export class IdeAnalyzer { const openFiles = new Set(this.openFiles); for (const failure of await this._analyzer.analyzeFiles([...openFiles])) { - if (failure.diagnostic != null) { + if (failure.diagnostic !== undefined) { addDiagnostic(failure.diagnostic); } - if (failure.diagnostics != null) { + if (failure.diagnostics !== undefined) { for (const diagnostic of failure.diagnostics) { addDiagnostic(diagnostic); } @@ -159,8 +159,8 @@ export class IdeAnalyzer { } = scriptInfo; if ( scriptInfo.kind === 'scripts-section-script' && - scriptNode != null && - wireitConfigNode == null + scriptNode !== undefined && + wireitConfigNode === undefined ) { const edit = getEdit(packageJson.jsonFile, [ {path: ['scripts', name], value: 'wireit'}, @@ -175,7 +175,10 @@ export class IdeAnalyzer { edit, }); } - if (scriptInfo.kind === 'wireit-section-script' && scriptNode == null) { + if ( + scriptInfo.kind === 'wireit-section-script' && + scriptNode === undefined + ) { const edit = getEdit(packageJson.jsonFile, [ {path: ['scripts', script.name], value: 'wireit'}, ]); @@ -194,8 +197,8 @@ export class IdeAnalyzer { } if ( - scriptNode == null || - wireitConfigNode == null || + scriptNode === undefined || + wireitConfigNode === undefined || scriptNode.value === 'wireit' ) { return codeActions; @@ -294,7 +297,7 @@ export class IdeAnalyzer { const dep = scriptInfo.dependency; const targetFile = dep.config.declaringFile; const targetNode = dep.config.configAstNode ?? dep.config.scriptAstNode; - if (targetFile == null || targetNode == null) { + if (targetFile === undefined || targetNode === undefined) { return; } @@ -305,7 +308,7 @@ export class IdeAnalyzer { return [ { originSelectionRange: sourceConverter.toIdeRange( - scriptInfo.dependency.astNode + scriptInfo.dependency.specifier ), targetUri: url.pathToFileURL(targetFile.path).toString(), targetRange: targetConverter.toIdeRange( @@ -368,7 +371,7 @@ export class IdeAnalyzer { packageDir: pathlib.dirname(packageJson.jsonFile.path), }); for (const dep of script.dependencies ?? []) { - if (offsetInsideRange(offset, dep.astNode)) { + if (offsetInsideRange(offset, dep.specifier)) { return { kind: 'dependency' as const, dependency: dep, diff --git a/src/language-server.ts b/src/language-server.ts index 9e62f268a..39276f798 100644 --- a/src/language-server.ts +++ b/src/language-server.ts @@ -118,7 +118,7 @@ documents.onDidClose((change) => { connection.onCodeAction(async (params) => { const document = documents.get(params.textDocument.uri); - if (document == null) { + if (document === undefined) { return []; } const path = url.fileURLToPath(document.uri); diff --git a/src/test/basic.test.ts b/src/test/basic.test.ts index f6b65f512..8399cabdd 100644 --- a/src/test/basic.test.ts +++ b/src/test/basic.test.ts @@ -363,6 +363,53 @@ test( }) ); +test( + 'cross-package dependency using object format', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + const cmdB = await rig.newCommand(); + await rig.write({ + 'foo/package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + dependencies: [ + { + script: '../bar:b', + }, + ], + }, + }, + }, + 'bar/package.json': { + scripts: { + b: 'wireit', + }, + wireit: { + b: { + command: cmdB.command, + }, + }, + }, + }); + const exec = rig.exec('npm run a', {cwd: 'foo'}); + + const invB = await cmdB.nextInvocation(); + invB.exit(0); + + const invA = await cmdA.nextInvocation(); + invA.exit(0); + + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 1); + assert.equal(cmdB.numInvocations, 1); + }) +); + test( 'cross-package dependency that validly cycles back to the first package', timeout(async ({rig}) => { @@ -996,4 +1043,97 @@ for (const agent of ['npm', 'yarn', 'pnpm']) { ); } +test( + 'triggersRerun:false dependency does not inherit fingerprint', + timeout(async ({rig}) => { + // a --[triggersRerun:false]--> b --> c + const a = await rig.newCommand(); + const b = await rig.newCommand(); + const c = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + b: 'wireit', + c: 'wireit', + }, + wireit: { + a: { + command: a.command, + dependencies: [ + { + script: 'b', + triggersRerun: false, + }, + ], + files: ['inputs/a'], + output: [], + }, + b: { + command: b.command, + dependencies: ['c'], + files: ['inputs/b'], + output: [], + }, + c: { + command: c.command, + files: ['inputs/c'], + output: [], + }, + }, + }, + }); + + // Initially everything runs. + console.log(0); + { + await rig.write('inputs/a', 'v1'); + await rig.write('inputs/b', 'v1'); + await rig.write('inputs/c', 'v1'); + const wireit = rig.exec('npm run a'); + (await c.nextInvocation()).exit(0); + (await b.nextInvocation()).exit(0); + (await a.nextInvocation()).exit(0); + assert.equal((await wireit.exit).code, 0); + assert.equal(a.numInvocations, 1); + assert.equal(b.numInvocations, 1); + assert.equal(c.numInvocations, 1); + } + + // Changing input of B re-runs B but not A. + { + await rig.write('inputs/b', 'v2'); + const wireit = rig.exec('npm run a'); + (await b.nextInvocation()).exit(0); + assert.equal((await wireit.exit).code, 0); + assert.equal(a.numInvocations, 1); + assert.equal(b.numInvocations, 2); + assert.equal(c.numInvocations, 1); + } + + // Changing input of C re-runs B and C but not A. + { + await rig.write('inputs/c', 'v2'); + const wireit = rig.exec('npm run a'); + (await c.nextInvocation()).exit(0); + (await b.nextInvocation()).exit(0); + assert.equal((await wireit.exit).code, 0); + assert.equal(a.numInvocations, 1); + assert.equal(b.numInvocations, 3); + assert.equal(c.numInvocations, 2); + } + + // Changing input of A re-runs A (just to be sure!). + { + await rig.write('inputs/a', 'v2'); + const wireit = rig.exec('npm run a'); + (await a.nextInvocation()).exit(0); + assert.equal((await wireit.exit).code, 0); + assert.equal(a.numInvocations, 2); + assert.equal(b.numInvocations, 3); + assert.equal(c.numInvocations, 2); + } + }) +); + test.run(); diff --git a/src/test/codeactions.test.ts b/src/test/codeactions.test.ts index 6c1ef4496..6a8b16de0 100644 --- a/src/test/codeactions.test.ts +++ b/src/test/codeactions.test.ts @@ -124,7 +124,7 @@ function applyEdit( before: string, action: CodeAction ): string { - if (action.edit == null) { + if (action.edit === undefined) { throw new Error(`Action ${action.title} had no edit`); } const edit = action.edit; @@ -132,7 +132,7 @@ function applyEdit( const filename = rig.resolve('package.json'); assert.equal(Object.keys(edit?.changes ?? {}), [filename]); const textEdits = edit?.changes?.[filename]; - if (textEdits == null) { + if (textEdits === undefined) { throw new Error(`Action ${action.title} had no edits for ${filename}`); } const converter = OffsetToPositionConverter.createUncachedForTest(before); diff --git a/src/test/errors-analysis.test.ts b/src/test/errors-analysis.test.ts index 0a69a5552..4ce5d0a17 100644 --- a/src/test/errors-analysis.test.ts +++ b/src/test/errors-analysis.test.ts @@ -173,13 +173,40 @@ test( checkScriptOutput( done.stderr, ` -❌ package.json:8:9 Expected a string, but was array. +❌ package.json:8:9 Expected a string or object, but was array. [] ~~` ); }) ); +test(`dependencies.script is not a string (object form)`, async ({rig}) => { + await rig.write('package.json', { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + dependencies: [ + { + script: [], + }, + ], + }, + }, + }); + const execResult = rig.exec(`npm run a`); + const done = await execResult.exit; + assert.equal(done.code, 1); + checkScriptOutput( + done.stderr, + ` +❌ package.json:9:21 Expected a string, but was array. + "script": [] + ~~` + ); +}); + test( 'dependency is empty or blank', timeout(async ({rig}) => { @@ -208,6 +235,66 @@ test( }) ); +test(`dependencies.script is empty or blank (object form)`, async ({rig}) => { + await rig.write('package.json', { + scripts: { + a: 'wireit', + 1: 'wireit', + }, + wireit: { + a: { + command: 'true', + dependencies: [ + { + script: '', + }, + ], + }, + 1: { + command: 'true', + }, + }, + }); + const execResult = rig.exec(`npm run a`); + const done = await execResult.exit; + assert.equal(done.code, 1); + checkScriptOutput( + done.stderr, + ` +❌ package.json:14:21 Expected this field to be nonempty + "script": "" + ~~` + ); +}); + +test(`dependencies.script is missing (object form)`, async ({rig}) => { + await rig.write('package.json', { + scripts: { + a: 'wireit', + 1: 'wireit', + }, + wireit: { + a: { + command: 'true', + dependencies: [{}], + }, + 1: { + command: 'true', + }, + }, + }); + const execResult = rig.exec(`npm run a`); + const done = await execResult.exit; + assert.equal(done.code, 1); + checkScriptOutput( + done.stderr, + ` +❌ package.json:13:9 Dependency object must set a "script" property. + {} + ~~` + ); +}); + test( 'command is not a string', timeout(async ({rig}) => { @@ -644,6 +731,39 @@ test( }) ); +test( + 'missing cross package dependency (object form)', + timeout(async ({rig}) => { + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + dependencies: [{script: './child:missing'}], + }, + }, + }, + 'child/package.json': { + scripts: {}, + }, + }); + const result = rig.exec('npm run a'); + const done = await result.exit; + assert.equal(done.code, 1); + checkScriptOutput( + done.stderr, + ` +❌ package.json:9:30 Cannot find script named "missing" in package "${rig.resolve( + 'child' + )}" + "script": "./child:missing" + ~~~~~~~` + ); + }) +); + test( 'missing same-package dependency with colon in name', timeout(async ({rig}) => { @@ -1833,4 +1953,41 @@ test( }) ); +test( + 'dependencies.triggersRerun is not a boolean', + timeout(async ({rig}) => { + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + b: 'wireit', + }, + wireit: { + a: { + dependencies: [ + { + script: 'b', + triggersRerun: 1, + }, + ], + }, + b: { + command: 'true', + }, + }, + }, + }); + const result = rig.exec('npm run a'); + const done = await result.exit; + assert.equal(done.code, 1); + checkScriptOutput( + done.stderr, + ` +❌ package.json:11:28 The "triggersRerun" property must be either true or false. + "triggersRerun": 1 + ~` + ); + }) +); + test.run(); diff --git a/src/test/ide.test.ts b/src/test/ide.test.ts index c8cbc3fb1..6ab69b90f 100644 --- a/src/test/ide.test.ts +++ b/src/test/ide.test.ts @@ -174,7 +174,7 @@ async function assertDefinition( options.contentsWithPipe.slice(offset + 1); ide.setOpenFileContents(options.path, contents); const sourceFile = await ide.getPackageJsonForTest(options.path); - if (sourceFile == null) { + if (sourceFile === undefined) { throw new Error(`could not get source file`); } const sourceConverter = OffsetToPositionConverter.get(sourceFile.jsonFile); @@ -201,7 +201,7 @@ async function assertDefinition( const targetFile = await ide.getPackageJsonForTest( url.fileURLToPath(definition.targetUri) ); - if (targetFile == null) { + if (targetFile === undefined) { throw new Error(`Could not load target file`); } const targetConverter = OffsetToPositionConverter.get(targetFile.jsonFile); @@ -225,7 +225,7 @@ async function assertDefinition( targetSelectionSquiggle, options.expected.targetSelection ); - if (definition.originSelectionRange == null) { + if (definition.originSelectionRange === undefined) { throw new Error(`No iriginSelectionRange returned`); } const sourceSelectionSquiggle = drawSquiggle( @@ -322,6 +322,39 @@ test(`we jump to the scripts section for a vanilla script`, async ({rig}) => { }); }); +test('jump to definition from object style dependency', async ({rig}) => { + const ide = new IdeAnalyzer(); + await assertDefinition(ide, { + path: rig.resolve('package.json'), + contentsWithPipe: JSON.stringify( + { + scripts: { + a: 'wireit', + b: 'echo', + }, + wireit: { + a: { + dependencies: [{script: '|b'}], + }, + }, + }, + null, + 2 + ), + expected: { + target: ` + "b": "echo" + ~~~~~~~~~~~`, + targetSelection: ` + "b": "echo" + ~~~`, + originSelection: ` + "script": "b" + ~~~`, + }, + }); +}); + test(`we don't get definitions for non-dep locations`, async ({rig}) => { const ide = new IdeAnalyzer(); await assertDefinition(ide, { diff --git a/src/test/json-schema.test.ts b/src/test/json-schema.test.ts index afab20989..4c1428932 100644 --- a/src/test/json-schema.test.ts +++ b/src/test/json-schema.test.ts @@ -58,6 +58,16 @@ test('a script with just dependencies is valid', () => { shouldValidate({wireit: {a: {dependencies: ['b']}}}); }); +test('dependency object is valid', () => { + shouldValidate({wireit: {a: {dependencies: [{script: 'b'}]}}}); +}); + +test('dependency object with triggersRerun:false annotation is valid', () => { + shouldValidate({ + wireit: {a: {dependencies: [{script: 'b', triggersRerun: false}]}}, + }); +}); + // I couldn't figure out how to make this test pass while keeping the other // error messages reasonable. // It just turned all errors into this one. @@ -72,7 +82,7 @@ test('a script with all fields set is valid', () => { wireit: { a: { command: 'b', - dependencies: ['c'], + dependencies: ['c', {script: 'c', triggersRerun: false}], files: ['d'], output: ['e'], clean: true, @@ -122,6 +132,80 @@ test('clean can be either a boolean or the string if-file-deleted', () => { ); }); +test('command must not be empty', () => { + expectValidationErrors( + { + wireit: { + a: { + command: '', + }, + }, + }, + ['instance.wireit.a.command does not meet minimum length of 1'] + ); +}); + +test('dependencies[i] must not be empty', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'true', + dependencies: [''], + }, + }, + }, + // TODO(aomarks) Can we get a better error message? Seems like the built-in + // toString() doesn't recurse, so we'd have to build the whole error message + // ourselves. + [ + 'instance.wireit.a.dependencies[0] is not any of [subschema 0],[subschema 1]', + ] + ); +}); + +test('files[i] must not be empty', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'true', + files: [''], + }, + }, + }, + ['instance.wireit.a.files[0] does not meet minimum length of 1'] + ); +}); + +test('output[i] must not be empty', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'true', + output: [''], + }, + }, + }, + ['instance.wireit.a.output[0] does not meet minimum length of 1'] + ); +}); + +test('packageLocks[i] must not be empty', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'true', + packageLocks: [''], + }, + }, + }, + ['instance.wireit.a.packageLocks[0] does not meet minimum length of 1'] + ); +}); + test('dependencies must be an array of strings', () => { expectValidationErrors( { @@ -144,7 +228,41 @@ test('dependencies must be an array of strings', () => { }, }, }, - ['instance.wireit.a.dependencies[0] is not of a type(s) string'] + [ + 'instance.wireit.a.dependencies[0] is not any of [subschema 0],[subschema 1]', + ] + ); +}); + +test('dependencies[i].script is required', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'b', + dependencies: [{}], + }, + }, + }, + [ + 'instance.wireit.a.dependencies[0] is not any of [subschema 0],[subschema 1]', + ] + ); +}); + +test('dependencies[i].triggersRerun must be boolean', () => { + expectValidationErrors( + { + wireit: { + a: { + command: 'b', + dependencies: [{script: 'b', triggersRerun: 1}], + }, + }, + }, + [ + 'instance.wireit.a.dependencies[0] is not any of [subschema 0],[subschema 1]', + ] ); }); diff --git a/src/test/parallelism.test.ts b/src/test/parallelism.test.ts index 4e34a6e7e..4cb927eae 100644 --- a/src/test/parallelism.test.ts +++ b/src/test/parallelism.test.ts @@ -246,7 +246,7 @@ test( // Start up simultaneous Wireit invocations for the same // script. - const concurrency = 25; + const concurrency = 10; const wireits = []; for (let i = 0; i < concurrency; i++) { wireits.push(rig.exec('npm run a')); diff --git a/src/test/service.test.ts b/src/test/service.test.ts index 3753c30e3..516221399 100644 --- a/src/test/service.test.ts +++ b/src/test/service.test.ts @@ -960,4 +960,84 @@ test( }) ); +test( + 'service with triggersRerun:false does not require restart in watch mode', + timeout(async ({rig}) => { + // service + // / \ + // v v + // hard soft + const service = await rig.newCommand(); + const hard = await rig.newCommand(); + const soft = await rig.newCommand(); + await rig.writeAtomic({ + 'package.json': { + scripts: { + service: 'wireit', + hard: 'wireit', + soft: 'wireit', + }, + wireit: { + service: { + command: service.command, + service: true, + dependencies: [ + 'hard', + { + script: 'soft', + triggersRerun: false, + }, + ], + }, + hard: { + command: hard.command, + files: ['input/hard'], + output: [], + }, + soft: { + command: soft.command, + files: ['input/soft'], + output: [], + }, + }, + }, + }); + + // Initial run + await rig.write('input/hard', '1'); + await rig.write('input/soft', '1'); + const wireit = rig.exec('npm run service --watch'); + const hardInv1 = await hard.nextInvocation(); + const softInv1 = await soft.nextInvocation(); + hardInv1.exit(0); + softInv1.exit(0); + const serviceInv1 = await service.nextInvocation(); + await wireit.waitForLog(/Service started/); + await wireit.waitForLog(/Watching for file changes/); + + // Changing input of soft dependency does not restart service + await rig.write('input/soft', '2'); + const softInv2 = await soft.nextInvocation(); + softInv2.exit(0); + await wireit.waitForLog(/Watching for file changes/); + assert.ok(serviceInv1.isRunning); + + // Changing input of hard dependency does restart service + await rig.write('input/hard', '2'); + const hardInv2 = await hard.nextInvocation(); + hardInv2.exit(0); + await serviceInv1.closed; + await service.nextInvocation(); + await wireit.waitForLog(/Service stopped/); + await wireit.waitForLog(/Service started/); + await wireit.waitForLog(/Watching for file changes/); + + wireit.kill(); + await wireit.exit; + assert.equal(service.numInvocations, 2); + assert.equal(hard.numInvocations, 2); + assert.equal(soft.numInvocations, 2); + }) +); + test.run(); diff --git a/src/test/util/package-json.ts b/src/test/util/package-json.ts index d017dc814..efcc1fbd0 100644 --- a/src/test/util/package-json.ts +++ b/src/test/util/package-json.ts @@ -16,7 +16,7 @@ export interface PackageJson { wireit?: { [scriptName: string]: { command?: string; - dependencies?: string[]; + dependencies?: Array; files?: string[]; output?: string[]; clean?: boolean | 'if-file-deleted'; diff --git a/src/util/ast.ts b/src/util/ast.ts index 516c81362..7c7646078 100644 --- a/src/util/ast.ts +++ b/src/util/ast.ts @@ -74,7 +74,7 @@ export function findNamedNodeAtLocation( return {ok: true, value: undefined}; } const name = parent.children?.[0]; - if (parent.type !== 'property' || name == null) { + if (parent.type !== 'property' || name === undefined) { return { ok: false, error: { diff --git a/src/util/package-json.ts b/src/util/package-json.ts index c49dbd5e1..62671504c 100644 --- a/src/util/package-json.ts +++ b/src/util/package-json.ts @@ -87,9 +87,9 @@ export class PackageJson { } } - private _getOrMakeScriptInfo(name: string): ScriptSyntaxInfo { + _getOrMakeScriptInfo(name: string): ScriptSyntaxInfo { let info = this._scripts.get(name); - if (info == null) { + if (info === undefined) { info = {name}; this._scripts.set(name, info); } @@ -114,11 +114,11 @@ export class PackageJson { return; } const scriptsSection = scriptsSectionResult.value; - if (scriptsSection == null) { + if (scriptsSection === undefined) { return; } const fail = failUnlessJsonObject(scriptsSection, this.jsonFile); - if (fail != null) { + if (fail !== undefined) { failures.push(fail); return; } @@ -126,10 +126,10 @@ export class PackageJson { if (child.type !== 'property') { continue; } - const [rawName, rawValue] = child.children ?? []; - if (rawName == null || rawValue == null) { + if (child.children === undefined) { continue; } + const [rawName, rawValue] = child.children; const nameResult = failUnlessNonBlankString(rawName, this.jsonFile); if (!nameResult.ok) { failures.push(nameResult.error); @@ -169,11 +169,11 @@ export class PackageJson { return; } const wireitSection = wireitSectionResult.value; - if (wireitSection == null) { + if (wireitSection === undefined) { return; } const fail = failUnlessJsonObject(wireitSection, this.jsonFile); - if (fail != null) { + if (fail !== undefined) { failures.push(fail); return; } @@ -181,17 +181,17 @@ export class PackageJson { if (child.type !== 'property') { continue; } - const [rawName, rawValue] = child.children ?? []; - if (rawName == null || rawValue == null) { + if (child.children === undefined) { continue; } + const [rawName, rawValue] = child.children ?? []; const nameResult = failUnlessNonBlankString(rawName, this.jsonFile); if (!nameResult.ok) { failures.push(nameResult.error); continue; } const fail = failUnlessJsonObject(rawValue, this.jsonFile); - if (fail != null) { + if (fail !== undefined) { failures.push(fail); continue; }