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

validate after migration. #173

Closed
Hiroshiba opened this issue Jan 18, 2023 · 11 comments
Closed

validate after migration. #173

Hiroshiba opened this issue Jan 18, 2023 · 11 comments

Comments

@Hiroshiba
Copy link

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.

@tim661811
Copy link

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.

@sindresorhus
Copy link
Owner

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
Copy link
Owner

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.

@Hiroshiba
Copy link
Author

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?

@sindresorhus
Copy link
Owner

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

@Hiroshiba
Copy link
Author

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.

@sindresorhus
Copy link
Owner

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?

@Hiroshiba
Copy link
Author

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

@uSkizzik
Copy link

uSkizzik commented Mar 3, 2023

Sooo what happened with this?

@tim661811
Copy link

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

@dopry
Copy link
Contributor

dopry commented Jun 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants