Skip to content

Commit

Permalink
Add "default" Option for "external" Environment Vars (#989)
Browse files Browse the repository at this point in the history
There may be cases where a caller doesn't want to pass an "external" environment variable. In those cases, rather than relying on the script to handle the default case, wireit can support a "default" option. This option is used when no value is passed for an "external" variable.
  • Loading branch information
ObliviousHarmony authored Jan 9, 2024
1 parent c561e14 commit 8560882
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Added

- Added a `default` option to the `env` setting for externally-provided environment variables to use when no value is provided.

### Changed

- The default logger for non-interactive environments has been switched to the 'quiet-ci' logger.
Expand Down
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ If an environment variable affects the behavior of a script but is set
_externally_ (i.e. it is passed to the `wireit` parent process), set the `env`
property to `{"external": true}`. This tells Wireit that if the value of an
environment variable changes across executions of a script, then its output
should not be re-used.
should not be re-used. You may also set a `default` value for the variable
to use when none is provided externally.

```json
{
Expand All @@ -477,6 +478,10 @@ should not be re-used.
"env": {
"MY_VARIABLE": {
"external": true
},
"MY_VARIABLE_2": {
"external": true,
"default": "foo"
}
}
}
Expand Down Expand Up @@ -831,6 +836,7 @@ The following properties can be set inside `wireit.<script>` objects in
| `clean` | `boolean \| "if-file-deleted"` | `true` | [Delete output files before running](#cleaning-output). |
| `env` | `Record<string, string \| object>` | `false` | [Environment variables](#environment-variables) to set when running this command, or that are external and affect the behavior. |
| `env[i].external` | `true \| undefined` | `undefined` | `true` if an [environment variable](#environment-variables) is set externally and affects the script's behavior. |
| `env[i].default` | `string \| undefined` | `undefined` | Default value to use when an external [environment variable](#environment-variables) is not provided. |
| `service` | `boolean` | `false` | [Whether this script is long-running, e.g. a server](#cleaning-output). |
| `packageLocks` | `string[]` | `['package-lock.json']` | [Names of package lock files](#package-locks). |

Expand Down
4 changes: 4 additions & 0 deletions schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@
"external": {
"markdownDescription": "Do not re-use output if this externally-provided environment variable changes across iterations.\n\nFor more info see: https://github.com/google/wireit#indicating-external-environment-variables",
"const": true
},
"default": {
"markdownDescription": "A default value to use when the environment variable is not provided externally.\n\nFor more info see: https://github.com/google/wireit#indicating-external-environment-variables",
"type": "string"
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions src/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1421,9 +1421,31 @@ export class Analyzer {
});
continue;
}
const defaultNode = findNodeAtLocation(val, ['default']);
if (defaultNode && defaultNode.type !== 'string') {
placeholder.failures.push({
type: 'failure',
reason: 'invalid-config-syntax',
script: placeholder,
diagnostic: {
severity: 'error',
message: 'Expected "default" to be a string',
location: {
file: packageJson.jsonFile,
range: {
length: (defaultNode ?? val).length,
offset: (defaultNode ?? val).offset,
},
},
},
});
continue;
}
const envValue = process.env[keyStr];
if (envValue !== undefined) {
entries.push([keyStr, envValue]);
} else if (defaultNode) {
entries.push([keyStr, defaultNode.value as string]);
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,10 @@ test(
BAR: {
external: true,
},
QUX: {
external: true,
default: 'qux-good',
},
},
},
},
Expand All @@ -1284,6 +1288,7 @@ test(
assert.equal(env.FOO, 'foo-good');
assert.equal(env.BAR, 'bar-good');
assert.equal(env.BAZ, 'baz-good');
assert.equal(env.QUX, 'qux-good');
inv.exit(0);
assert.equal((await wireit.exit).code, 0);
}),
Expand Down
34 changes: 34 additions & 0 deletions src/test/errors-analysis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,40 @@ test(
}),
);

test(
'env entry value that is object with "default" property must be a string',
rigTest(async ({rig}) => {
await rig.write({
'package.json': {
scripts: {
a: 'wireit',
},
wireit: {
a: {
command: 'true',
env: {
FOO: {
external: true,
default: {},
},
},
},
},
},
});
const result = rig.exec('npm run a');
const done = await result.exit;
assert.equal(done.code, 1);
checkScriptOutput(
done.stderr,
`
❌ package.json:11:22 Expected "default" to be a string
"default": {}
~~`,
);
}),
);

test(
"script with no command can't have env",
rigTest(async ({rig}) => {
Expand Down

0 comments on commit 8560882

Please sign in to comment.