const is validated by fjs, add strict mode

[Uzlopak]

This is what I would expect from fast-json-stringify. Probably needs heavy discussion as it is heavily breaking change.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[ivan-tymoshenko]

@Uzlopak and everyone else. I finished a discussion with @Eomm on const question, because it's an edge case that shows us how we should design fjs in the future. You can find some thoughts there and join the discussion.
#532 (reply in thread)

[Uzlopak]

@mcollina

Before i finish this PR i want to come to a conclusion. I know it is not viable. I would actually add a deep compare function for that.

@ivan-tymoshenko

I know that this is basically the case were we always come to the conclusion on what the purpose of this PR is.

Here i purposefully also used ajv validator as compare function instead the is-my-json-valid, to show that we diverge from the behavior significantly. E.g.

{
  type: 'object',
  properties: {
    foo: { type: 'string', nullable: true, const: 'bar'
  }
}

If we use this schema and use following payload:

{ foo: null }

Then JSON.stringify returns {"foo":null} , ajv throws an error and fast-json-stringify returns {"foo":"bar"}.

Imho fjs has to either return the same value as json.stringify or need to throw an error.

The current behaviour can imho not be correct.

[Eomm]

I think we have different visions that could be summarized to:

1. FJS should validate the input and throw an error if the input doesn't match the json-schema
2. FJS should NOT validate** the input and should try to serialize it by following the json-schema
3. FJS should NOT validate** the input and should ignore some json-schema keywords (such as const)

Did I miss some cases?

** even if the produced string output does not match the json-schema

[Uzlopak]

Yeah basically.

[Uzlopak]

WELL!

I wrote a function which based on the const value generates validation code. I oriented myself on fast-deep-equal. Basically it generates branchless code for validating the input.

Maybe this helps to expedite the discussion regarding #532

[ivan-tymoshenko]

From my point of view trying to validate all types and throwing an error it's the wrong way. You will inevitably run into validation problems. Why do constants throw an error while other keywords don't? I think that we should use constants when it can speed up serialization, and if the value is not equal to a constant we should fallback to default serialization by type.

[Uzlopak]

I would rather build a serializer which completly validates the input based on the json schema than have a serializer, where I provide a whole schema and in the end only a small part of the schema is actually used. When I write that a field of an input-object has a const value of x, then I expect that it is set accordingly on the input and doesnt get serialized anyway nor how it is done currently with a value which is not set.

[ivan-tymoshenko]

The problem is that we can't build a serializer + validator without having a significant performance drop. Yes, it might work faster than serializer and validator separately, but it will work much slower than the serializer by itself. Basically, you want to rewrite ajv and merge it with fjs, right?

[Uzlopak]

Maybe it makes sense, to design all those issues as "plugins"?

So instead of having here the big discussion regarding what the goal should be, we could have another option validate where we allow to enable or disable this or that json schema keyword?

So if somebody wants to enable maxLength or minLength validation, just set validate.minLength or validate.maxLength accordingly to true.

[ivan-tymoshenko]

Yes, I'm ok with a solution like that. You can see that in the discussion with @Eomm, we discussed "strict" and "non-strict" versions of fjs. It has something similar with what you suggest. The questions are:

1. What version should be the default for Fastify?
2. We need to build the plugin system in such a way that it doesn't turn the code into a mess of conditions. If we make a successful system of dynamically linked plugins, this will be a good solution.

[Uzlopak]

@ivan-tymoshenko

In #600 I encapsulate the whole context in a variable. If that gets merged, I could instead of in line 859 if (schema.const !== undefined) { do something like if (context.options && (context.options.strict || context.options.validate.const) && schema.const !== undefined) {

[Eomm]

we are validating our test code here

Suggested change

	t.ok(validate((input)))
	t.ok(validate(JSON.parse(output)))

[Eomm]

As discussed, I agree to throw behind an option or plugin

in general, my opinion with FJS, it should never throw/validate

[Uzlopak]

if it does not throw, what should it then serialize? just doing JSON.stringify(input) ?

[Eomm]

we should validate the fjs output

[Uzlopak]

I am currently trying to implement some other keywords like

• enum
• maxProperties
• minProperties
• maxLength
• minLength
• maxItems
• minItems
• minimum
• maximum

Just to get the feeling on how we can implement it properly. Already enum is pain in the ass, because somehow i get an additional } but i dont know why. Lol.
Fixing that and i could push the commit.

But looks good so far.

[Uzlopak]

I thouht alot about this. I think the biggest issue is, that if we have a big array or object, than the generated validator will be vey big too. So I wonder how ajv solves this issue. But how fast-json-stringify does with const is also very wrong :/

[Uzlopak]

ajv basically stores the schema and builds the validator based on the schema. So it is basically ok, what i implemented here. Maybe it makes sense to optimize further for arrays.

[Uzlopak]

By having the context now passed through, it was easy to implement a strict mode.

[ivan-tymoshenko]

Defining features such as strict mode is an important decision for a library. If you really want to add it, you should think very carefully so as not to create undesirable consequences.

1. What is a strict mode? What guarantees I can get as a user using strict mode? What checks should it include? What checks should it not include?
2. The same questions for the unstrict mode.
3. What mode should be default for the Fastiy?

[Uzlopak]

I am not saying that this PR is ready. I am just shuffling the code to determine the next step.

E.g. @Eomm wrote that it we should not throw. But required keyword is throwing. So should we make required non-throwing?

[ivan-tymoshenko]

It depends on what you want to archive. I don't understand. That is why I'm asking. It's hard to say the correct answer without input conditions. Everyone might understand strict and unstrict modes differently. What are your definitions of strict and unstrict mode?

[Uzlopak]

@ivan-tymoshenko
We can even do it like tsconfig and have a global strict setting and e.g. strictRequired, strictConst, strictX.

@Eomm
coercion and strictness... I think coercion is independent of strictness.

[ivan-tymoshenko]

Ok, I think that idea of configuring rules is a good compromise. I think we should create another issue and discuss it there.

[ivan-tymoshenko]

Another part of the question is how the unstrict version of FJS should work. When it should throw an error, when it should ignore some input data, etc.

[Uzlopak]

Any body who wants to work on this further is invited to do so.

But closing it as at the current time there is no eta nor a active discussion.

[Eomm]

But closing it as at the current time there is no eta nor a active discussion.

Discussion is not a real-time chat, but it could take weeks or months.

I don't want to be the one who says "no" nor stops the progress, but AFAIR, I did not read any proposed plan - did I miss it?

[ivan-tymoshenko]

A new @Uzlopak idea: incremental validation: #579 (comment). IMHO, this could be a compromise solution. It allows us to have an existing Fastify configuration. The question is how far we want to go with it. I don't really want to reinvent the Ajv.