fix(misc)!: only provide default value for object properties if object already has value (#28838)
Defaults causing an object to be defined causes some confusing behavior when a schema has defaults and required properties for a given object prop. If the required prop doesn't have a default value, and another property does, it becomes invalid to not pass the object when it would be valid had that default not been specified. BREAKING CHANGE: Currently if an executor's schema provides some default values for an object's properties, those defaults cause the object to be defined. This changes that, such that the defaults are only applied if the object exists in the first place. Fixes #23153
This commit is contained in:
Craigory Coppola
GitHub
parent
7b9add5582
commit
79b89fb7a7
@ -190,6 +190,43 @@ describe('params', () => {
|
||||
|
||||
expect(options).toEqual({});
|
||||
});
|
||||
|
||||
it('should not throw if missing required property of an optional object', () => {
|
||||
const commandLineOpts = {};
|
||||
const target: TargetConfiguration = {
|
||||
executor: '@nx/do:stuff',
|
||||
options: {},
|
||||
};
|
||||
|
||||
const options = combineOptionsForExecutor(
|
||||
commandLineOpts,
|
||||
'production',
|
||||
target,
|
||||
{
|
||||
properties: {
|
||||
foo: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
bar: {
|
||||
description: 'The target server ',
|
||||
type: 'string',
|
||||
},
|
||||
baz: {
|
||||
type: 'string',
|
||||
default: 'qux',
|
||||
},
|
||||
},
|
||||
required: ['bar'],
|
||||
},
|
||||
},
|
||||
required: [],
|
||||
},
|
||||
'proj',
|
||||
process.cwd()
|
||||
);
|
||||
|
||||
expect(options).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe('coerceTypes', () => {
|
||||
@ -595,7 +632,9 @@ describe('params', () => {
|
||||
|
||||
it('should be able to set defaults for underlying properties', () => {
|
||||
const opts = setDefaults(
|
||||
{},
|
||||
{
|
||||
a: {},
|
||||
},
|
||||
{
|
||||
properties: {
|
||||
a: {
|
||||
@ -798,6 +837,27 @@ describe('params', () => {
|
||||
).toThrow("Required property 'a' is missing");
|
||||
});
|
||||
|
||||
it('should not throw if missing a required property of an optional object', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
{},
|
||||
{
|
||||
properties: {
|
||||
a: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
b: {
|
||||
type: 'boolean',
|
||||
},
|
||||
},
|
||||
required: ['b'],
|
||||
},
|
||||
},
|
||||
}
|
||||
)
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should throw if none of the oneOf conditions are met', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
@ -962,13 +1022,20 @@ describe('params', () => {
|
||||
it('should throw if property name matching pattern is not valid', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
{ a: true, b: false },
|
||||
{
|
||||
a: true,
|
||||
b: false,
|
||||
},
|
||||
{
|
||||
properties: {
|
||||
a: { type: 'boolean' },
|
||||
a: {
|
||||
type: 'boolean',
|
||||
},
|
||||
},
|
||||
patternProperties: {
|
||||
'^b$': { type: 'number' },
|
||||
'^b$': {
|
||||
type: 'number',
|
||||
},
|
||||
},
|
||||
additionalProperties: false,
|
||||
}
|
||||
@ -978,63 +1045,6 @@ describe('params', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle properties matching patternProperties schema', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
{ a: true, b: false },
|
||||
{
|
||||
properties: {
|
||||
a: { type: 'boolean' },
|
||||
},
|
||||
patternProperties: {
|
||||
'^b$': { type: 'boolean' },
|
||||
},
|
||||
additionalProperties: false,
|
||||
}
|
||||
)
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should throw if additional property does not match schema', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
{ a: true, b: 'b', c: 'c' },
|
||||
{
|
||||
properties: {
|
||||
a: { type: 'boolean' },
|
||||
},
|
||||
patternProperties: {
|
||||
'^b$': { type: 'string' },
|
||||
},
|
||||
additionalProperties: {
|
||||
type: 'number',
|
||||
},
|
||||
}
|
||||
)
|
||||
).toThrow(
|
||||
"Property 'c' does not match the schema. 'c' should be a 'number'."
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle additional properties when they match the additionalProperties schema', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
{ a: true, b: 'b', c: 1, d: 2 },
|
||||
{
|
||||
properties: {
|
||||
a: { type: 'boolean' },
|
||||
},
|
||||
patternProperties: {
|
||||
'^b$': { type: 'string' },
|
||||
},
|
||||
additionalProperties: {
|
||||
type: 'number',
|
||||
},
|
||||
}
|
||||
)
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should throw if found unsupported positional property', () => {
|
||||
expect(() =>
|
||||
validateOptsAgainstSchema(
|
||||
@ -1870,16 +1880,25 @@ describe('params', () => {
|
||||
}
|
||||
);
|
||||
|
||||
expect(prompts).toEqual([
|
||||
// Limit is determined by terminal size
|
||||
// deleting it to make the snapshot consistent
|
||||
delete prompts[0].limit;
|
||||
|
||||
expect(prompts).toMatchInlineSnapshot(`
|
||||
[
|
||||
{
|
||||
message: 'What kind of pets do you have?',
|
||||
name: 'pets',
|
||||
type: 'multiselect',
|
||||
choices: ['cat', 'dog', 'fish'],
|
||||
limit: expect.any(Number),
|
||||
validate: expect.any(Function),
|
||||
"choices": [
|
||||
"cat",
|
||||
"dog",
|
||||
"fish",
|
||||
],
|
||||
"message": "What kind of pets do you have?",
|
||||
"name": "pets",
|
||||
"type": "multiselect",
|
||||
"validate": [Function],
|
||||
},
|
||||
]);
|
||||
]
|
||||
`);
|
||||
});
|
||||
|
||||
describe('Project prompts', () => {
|
||||
|
||||
@ -536,15 +536,13 @@ function setPropertyDefault(
|
||||
schema: any,
|
||||
definitions: Properties
|
||||
) {
|
||||
let defaultValueToSet: any | undefined;
|
||||
|
||||
if (schema.$ref) {
|
||||
schema = resolveDefinition(schema.$ref, definitions);
|
||||
}
|
||||
|
||||
if (schema.type !== 'object' && schema.type !== 'array') {
|
||||
if (opts[propName] === undefined && schema.default !== undefined) {
|
||||
opts[propName] = schema.default;
|
||||
}
|
||||
} else if (schema.type === 'array') {
|
||||
if (schema.type === 'array') {
|
||||
const items = schema.items || {};
|
||||
if (
|
||||
opts[propName] &&
|
||||
@ -555,20 +553,33 @@ function setPropertyDefault(
|
||||
setDefaultsInObject(valueInArray, items.properties || {}, definitions)
|
||||
);
|
||||
} else if (!opts[propName] && schema.default) {
|
||||
opts[propName] = schema.default;
|
||||
defaultValueToSet = schema.default;
|
||||
}
|
||||
} else {
|
||||
const wasUndefined = opts[propName] === undefined;
|
||||
if (wasUndefined) {
|
||||
// We need an object to set values onto
|
||||
opts[propName] = {};
|
||||
if (opts[propName] === undefined && schema.default !== undefined) {
|
||||
defaultValueToSet = schema.default;
|
||||
}
|
||||
|
||||
setDefaultsInObject(opts[propName], schema.properties || {}, definitions);
|
||||
if (schema.type === 'object') {
|
||||
const wasUndefined = opts[propName] === undefined;
|
||||
if (!wasUndefined) {
|
||||
setDefaultsInObject(
|
||||
opts[propName],
|
||||
schema.properties || {},
|
||||
definitions
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If the property was initially undefined but no properties were added, we remove it again instead of having an {}
|
||||
if (wasUndefined && Object.keys(opts[propName]).length === 0) {
|
||||
delete opts[propName];
|
||||
if (defaultValueToSet !== undefined) {
|
||||
try {
|
||||
validateProperty(propName, defaultValueToSet, schema, definitions);
|
||||
opts[propName] = defaultValueToSet;
|
||||
} catch (e) {
|
||||
// If the default value is invalid, we don't set it...
|
||||
// this should honestly never be needed... but some notable
|
||||
// 3rd party schema's are invalid.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user