sindresorhus/conf

`validate` after migration.

Hiroshiba opened this issue · 11 comments

The conf library does validate after migration.
validate

this._validate(store);

migration
this._migrate(options.migrations, options.projectVersion, options.beforeEachMigration);

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.

export type Migrations<T extends Record<string, any>> = Record<string, (store: Conf<T>) => void>;

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.

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.

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.

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.

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?

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

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.

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?

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

Sooo what happened with this?

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

@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.