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

Defaults not applied from schema #85

Open
YodaDaCoda opened this issue Aug 29, 2019 · 10 comments
Open

Defaults not applied from schema #85

YodaDaCoda opened this issue Aug 29, 2019 · 10 comments
Labels
bug 💵 Funded on Issuehunt This issue has been funded on Issuehunt help wanted

Comments

@YodaDaCoda
Copy link

YodaDaCoda commented Aug 29, 2019

Issuehunt badges

I'm using a schema to create my settings, and I've been unable to have conf or electron-config use the defaults provided in the schema.

I've included a reduced test case here:

const Conf = require('conf');

const schema = {
  fizz: {
    type: 'string',
    default: 'buzz',
  },
  foo: {
    type: 'object',
    properties: {
      bar: {
        type: 'string',
        default: 'baz',
      },
    },
  },
};

const config = new Conf({schema});

console.log(config.get('fizz'));    // undefined - should be 'buzz'
console.log(config.get('foo'));     // undefined - should be { bar: 'baz' }
console.log(config.get('foo.bar')); // undefined - should be 'baz'

console.assert(config.get('fizz') === 'buzz');   // Assertion failed
console.assert(config.get('foo.bar') === 'baz'); // Assertion failed

An rough solution to populate defaults from schema is below, but I wouldn't expect to have to do this when using a schema that includes defaults.

function setDefaults(schema, config, key_chain) {
  for (let [key, val] of Object.entries(schema)) {
    let this_key = (key_chain ? key_chain + '.' : '') + key;
    if (val.type === 'object') {
      return setDefaults(val.properties, config, this_key);
    }
    if (!config.has(key) && Object.hasOwnProperty.call(val, 'default')) {
      config.set(this_key, val.default);
    }
  }
}

IssueHunt Summary

Backers (Total: $40.00)

Become a backer now!

Or submit a pull request to get the deposits!

Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Owner

I'm not really sure what the problem is. We tell ajv to use the defaults here:

conf/index.js

Line 85 in 931ffce

useDefaults: true,

@issuehunt-oss
Copy link

issuehunt-oss bot commented Sep 9, 2019

@issuehunt has funded $40.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Sep 9, 2019
@sindresorhus sindresorhus changed the title Defaults not applied from Schema Defaults not applied from schema Sep 9, 2019
@yaodingyd
Copy link
Contributor

@YodaDaCoda I cannot reproduce the case with basic type(string). Can you give more details? (OS and node version)

About use object type with default, I believe this is the correct behavior according to ajv: default keywords will be ingored if not in properties or items subschemas. For foo, there is no default on its level, so it will be undefined. I use the following code and get the same result:

const Ajv = require('ajv');

const ajv = new Ajv({
		allErrors: true,
		format: 'full',
		useDefaults: true,
		errorDataPath: 'property'
 });

const schema = {
  type: "object",
  properties: {
	 	fizz: {
			type: "object",
			properties: {
				bar: {
					type: 'string',
					default: 'baz',
				},
			},
		},
	},
};


const data = {};

var validate = ajv.compile(schema);
console.log(validate(data)); // true
console.log(data); // { }

The best we can do is document this in readme.

@YodaDaCoda
Copy link
Author

@yaodingyd thanks, this has made me review my issue.

This is interesting. Adding default: {} to foo in my test above produces the desired results.

From what I can tell, draft 07 of the spec doesn't define the behaviour of a validator in this regard. Draft 08 adds some examples that happen to include handling of defaults, and seems to imply that the results I expected are correct (i.e. able to specify defaults on a sub-property without specifying that the property is default: {}).

This sounds like an ajv issue. Can anyone with a better understanding than myself weigh in?

@ghost
Copy link

ghost commented Oct 21, 2019

This is interesting. Adding default: {} to foo in my test above produces the desired results.

Isn't deault property of foo's bar property scoped? That is, as far as I understand, Ajv will assign default property bar to the foo object if the foo object exists but bar property does not.
That's why default property on foo level schema "works".

Here's an example and relevant section in Ajv docs.

@tunnckoCore
Copy link

@krnik true.

I think @YodaDaCoda is right in his expectation. When there is properties it's obviously expected that foo will/should be at least an empty {} object no matter what.

@YodaDaCoda: Adding default: {} to foo in my test above produces the desired results.

At least the work around is easy, haha.

Yup, sounds like problem in ajv and its logic.

@sindresorhus
Copy link
Owner

Yup, sounds like problem in ajv and its logic.

Can you open an issue on ajv and comment the link here?

@hovancik
Copy link

hovancik commented May 13, 2021

I'm having similar problem, not sure if it's related. I've had:

  const schema = {
    reminders: {
      type: 'array',
      default: [{
        uuid: uuidv4(),
        type: 'once',
        interval: 'every 10 seconds',
        keep: true,
        title: 'Welcome to LaterOn - The reminder app!',
        body: 'Learn more at https://LaterOn.app.'
      }]
    }
  }

Which worked fine, correct default value gets created.

Once I added another value to schema, it is not being added when I run the app again:

  const schema = {
    reminders: {
      type: 'array',
      default: [{
        uuid: uuidv4(),
        type: 'once',
        interval: 'every 10 seconds',
        keep: true,
        title: 'Welcome to LaterOn - The reminder app!',
        body: 'Learn more at https://LaterOn.app.'
      }]
    },
    openAtLogin: {
      type: 'boolean',
      default: false
    }
  }

Am I doing something wrong or? I don't have object as in previous examples, but simple boolean.

Interestingly, if I remove my store file, on the app restart only the first array default will get created, but not the second boolean. If I remove array altogether, boolean will get created. If I change the schema so that boolean is first, then both values get created.

So then I tried: put boolean first and see what happens when array already exists. Unfortunatelly that did not work, new default does not get created.

note: just noticed that this is for conf. I am using electron-store but I guess it makes no difference

@hovancik
Copy link

hey @sindresorhus I think this is indeed the bug in conf (or electron-store).

I've been playing with some workarounds here and elsewhere (where they check if store.has(key)) just to find that the key that I was missing (it did not get saved to the config file on app start) was existing and had correct value but was not saved to disk.

This seems to do the trick to save the file:

store.store = Object.assign({}, store.store)

I think that we need to save every time? not familiar with the code to say where the fix should go

		try {
			assert.deepEqual(fileStore, store);
		} catch (_) {
			this.store = store;
		}

@sushantdhiman
Copy link

ajv maintainers has decided that default will not work for nested properties, if top level property has no default value

This issue should be closed, as this library is using useDefaults from ajv for this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 💵 Funded on Issuehunt This issue has been funded on Issuehunt help wanted
Projects
None yet
Development

No branches or pull requests

6 participants