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

RFC: config validation #2408

Open
armano2 opened this issue Jan 15, 2021 · 7 comments
Open

RFC: config validation #2408

armano2 opened this issue Jan 15, 2021 · 7 comments

Comments

@armano2
Copy link
Contributor

armano2 commented Jan 15, 2021

Context

While working on #2398 I noticed that we are not validating if config provided has correct structure at all, in this pr i added basic validation for resolved configuration as doing unsafe type casting is really bad idea

Proposed implementation in not ideal, and requires more refining.

Proposal

Create new package responsible for validation of configuration and rules @commitlint/config-validator and use ajv to validate configurations loaded by @commitlint/load and @commitlint/resolve-extends

Validation should be done in 2 steps: config and rules

Config validation #2412

this should be first step of validation of config structure and type check properties, but it should not validate structure of specific rules as by default we should accept any configuration that matches

'string': [RuleConfigSeverity /* 0, 1, 2 */, RuleConfigCondition /* when */, unknown /* options */]

Rules validation

after resolving and merging all extended configs we should proceed to validate options for rules:
this is going to require change in structure of rule definition, we should add support for object and deprecate raw functions

in case if we don't have schema for rule, we should skip validation of options

Rule as object

schema property is optional, but recommended,

  • if provided we should validate options against it
  • if not provided we should threat options as any/unknown
{
  schema: { ... },
  handler (parsed, when = 'always', options) { } // I'm not sure about this name, but this can be refined/changed
}

Rule as function - backward compatibility

It should be threated as there is no schema provided and we should accept any options

function rule (parsed, when = 'always', options) {}

Why we should change default structure of rules

with moving away of simple function structure it gives us possibility:


this is my general idea for this, its not written in best way, but I'd like to hear your opinion on this

@escapedcat
Copy link
Member

escapedcat commented Jan 15, 2021

ajv is this, right?: https://ajv.js.org

@armano2
Copy link
Contributor Author

armano2 commented Jan 15, 2021

yes, its same package as eslint is currently using https://github.com/eslint/eslint/blob/75fea9bcdd3dde5a07e0089d9011a4df518cdbe3/package.json#L51

@armano2
Copy link
Contributor Author

armano2 commented Jan 15, 2021

i updated proposal a little, to make it more clear

@armano2
Copy link
Contributor Author

armano2 commented Jan 16, 2021

json schema, based on SchemaStore/schemastore#1443

https://json.schemastore.org/commitlintrc

@escapedcat
Copy link
Member

I believe this done because the PR was merged

@armano2
Copy link
Contributor Author

armano2 commented Mar 31, 2022

sadly no, this describes 2 part of config validation, and only first part has been implemented "Rules validation" is not yet implemented

@escapedcat
Copy link
Member

Ah, thanks! Was wondering for a moment...

@escapedcat escapedcat reopened this Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants