conf icon indicating copy to clipboard operation
conf copied to clipboard

`validate` after migration.

Open Hiroshiba opened this issue 2 years ago • 10 comments

The conf library does validate after migration. validate https://github.com/sindresorhus/conf/blob/184fc278736dee34c44d4e7fa7e1b2a16ffdd5be/source/index.ts#L133 migration https://github.com/sindresorhus/conf/blob/184fc278736dee34c44d4e7fa7e1b2a16ffdd5be/source/index.ts#L150

This will validate past configuration files with the latest ajv schema, which is prone to type errors. It would be nice if validate could be done after migration.

However, the solution did not seem easy. Ajv with useDefaults: true uses type checking and value assignment at the same time, and the conf library uses that specification. https://ajv.js.org/options.html#usedefaults

But the conf library passes a store instance during migration. https://github.com/sindresorhus/conf/blob/184fc278736dee34c44d4e7fa7e1b2a16ffdd5be/source/types.ts#L244

This leaves us stuck with a specification that validates past objects in the current schema.

My suggested approach, although it would be a destructive change, would be to pass a json object instead of a store instance during migration and return the next json object. That way we can validate with the last json object created and apply the latest schema to the latest object.

Hiroshiba avatar Jan 18 '23 03:01 Hiroshiba

I'm running Conf 6.0.0 and also came across this issue when introducing a new required property. The previous versions of my config didn't have this property yet, so the config is always invalid according to the schema.

tim661811 avatar Jan 24 '23 14:01 tim661811

My suggested approach, although it would be a destructive change, would be to pass a json object instead of a store instance during migration and return the next json object.

This would be a big breaking change. And you are only thinking of user using schema. There are probably many just using the defaults.

sindresorhus avatar Jan 31 '23 04:01 sindresorhus

I'm happy to consider other solutions. I was not the one that added this feature and I personally don't use it, so it's not something I have interest in working on.

sindresorhus avatar Jan 31 '23 04:01 sindresorhus

I see!

According to the Ajv documentation, there is an option to turn off validate. https://ajv.js.org/options.html#validateschema

How about specifying validateSchema="log" to disable validation, or setting a flag to control whether validation is performed?

Hiroshiba avatar Feb 01 '23 06:02 Hiroshiba

What do you think about disabling validation automatically during the migration process?

sindresorhus avatar Feb 01 '23 07:02 sindresorhus

Personally, I didn't think it would be much of a problem without validation. (Rather than not being able to migrate.)

Alternatively, it might be better to turn off validation, .validate, and then turn on validation and .validate again after the migration is complete.

Hiroshiba avatar Feb 01 '23 17:02 Hiroshiba

Alternatively, it might be better to turn off validation, .validate, and then turn on validation and .validate again after the migration is complete.

How is that different from my suggestion?

sindresorhus avatar Feb 04 '23 09:02 sindresorhus

Oops sorry, we had the same opinion! I misunderstood.

Hiroshiba avatar Feb 04 '23 21:02 Hiroshiba

Sooo what happened with this?

uSkizzik avatar Mar 03 '23 14:03 uSkizzik

What's the estimate of when this issue will be fixed?

tim661811 avatar Apr 13 '23 09:04 tim661811

@sindresorhus I just submitted #194. With regard to migration I think deferring validation until after migration is an ideal approach. When a schema is changed at next launch the old schema is not going to be valid. This means that generally the schema is immutable unless you want to setup an anyOf for every property in your schema with the old an new versions and then write a custom migration within your app code to maintain the schema in conf. What risk do you think there is in moving the migration code to before validation in the constructor.

dopry avatar Jun 21 '24 18:06 dopry