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

2.1.0 breaks TS build #39

Closed
mmouterde opened this issue Mar 22, 2020 · 21 comments · Fixed by #89
Closed

2.1.0 breaks TS build #39

mmouterde opened this issue Mar 22, 2020 · 21 comments · Fixed by #89

Comments

@mmouterde
Copy link

2.0.0 works great but once updated to 2.1.0 I got strange errors from ts compiler.

Type '{ type: string; required: string[]; properties: { firstName: { type: string; }; lastName: { type: string; }; email: { type: string; }; phone: { type: string[]; }; password: { type: string; }; participantId: { type: string; }; }; }' is not assignable to type 'ValidateFunction'.
  Type '{ type: string; required: string[]; properties: { firstName: { type: string; }; lastName: { type: string; }; email: { type: string; }; phone: { type: string[]; }; password: { type: string; }; participantId: { type: string; }; }; }' is not assignable to type 'JSONSchema7'.
    Types of property 'type' are incompatible.
      Type 'string' is not assignable to type '"string" | "number" | "boolean" | "object" | "integer" | "null" | "array" | JSONSchema7TypeName[]'.
@vacekj
Copy link
Owner

vacekj commented Mar 24, 2020

I will look into it ASAP.

@vacekj
Copy link
Owner

vacekj commented Mar 27, 2020

Can you please provide a minimal reproduction repo or example?

@mmouterde
Copy link
Author

sure https://github.com/mmouterde/bugDemo

import { Validator } from 'express-json-validator-middleware';
const UserParamSchemaForUpdate = {
    type: 'object',
    properties: {
        firstName: { type: 'string' }
    },
};
const validator = new Validator({
    allErrors: true,
    format: 'full',
    jsonPointers: true,
});
validator.validate({
    body: UserParamSchemaForUpdate,
});

@rmolinamir
Copy link

Basically body is not compatible with objects unless explicitly specified such as:

app.post('/template', validate({ body: schemas.templateCreate as ValidateFunction }), templateController.create_template);

@monooso
Copy link

monooso commented Sep 15, 2020

@vacekj I'm having the same problem.

@rmolinamir is correct in that the error is triggered by type: "object". His solution doesn't make any sense to me, unfortunately.

The strange thing is that passing an object directly to the validate function works. For example:

import { Validator } from 'express-json-validator-middleware'

const validator = new Validator({ allErrors: true })

const mySchema = { type: 'object', properties: {} }

// Passing the schema inline works
validator.validate({ body: { type: 'object', properties: {} } })

// Passing the variable throws an error
validator.validate({ body: mySchema })

// Even expanding the variable throws an error
validator.validate({ body: { ...mySchema } })

I'm completely stumped. Any thoughts?


Update

The following functions as a workaround.

import { JSONSchema7 } from 'json-schema'

validator.validate({ body: mySchema as JSONSchema7 })

@senorpedro
Copy link
Contributor

senorpedro commented Oct 23, 2020

I experienced the same issue, what worked for me was to put as const after the type value, e.g.

{
  type: 'object' as const
 ...
}

unfortunately this meant not being able to use .json files anymore, had to migrate them to TS

I think the advantage of this approach is that TS will complain when a invalid value for type is entered, which it won't do when explicit type casting to JSONSchema7.

@simonplend
Copy link
Collaborator

Hi folks 👋 I'm the new maintainer of this project.

I've just started looking into this issue and will attempt to reproduce it. I'll let you know how I get on, but any further insights as to what the issue might be are very welcome in the meantime!

@NGTOne
Copy link
Contributor

NGTOne commented Feb 17, 2021

It looks like this issue was actually introduced by adding types, period - 2.1.0 was the first package version to actually have type definitions, and they've changed little since then.

@simonplend
Copy link
Collaborator

@NGTOne That makes sense. I think this issue needs to be resolved as removing the types completely isn't a viable option. I'm hoping to have time to do some digging on this later this week.

@simonplend
Copy link
Collaborator

I've been trying to resolve this issue, but it's proving tricky. I've created a minimal reproducible example demonstrating the problem: https://bit.ly/3oIwQ9z (thanks to @monooso for sharing their workaround above).

@Beraliv
Copy link

Beraliv commented May 23, 2021

@simonplend You need to use as const to make object a string literal:

It works good here: https://tsplay.dev/WJ96lm

@monooso can you provide a minimal working example in CodeSandbox with json?

However, as I understand, even with "resolveJsonModule": true you will get string, but not the string literal
But you can convert *.json to *.ts files with as const and it will work just fine for you

@simonplend
Copy link
Collaborator

@Beraliv That's great, thank you! It seems similar to the solution which @senorpedro mentioned above.

This raises a couple of questions for me:

  • Why does making object a string literal fix things?
  • Is it preferable to put as const after the object string, or after the whole object itself?

I really appreciate everyone's help with this issue. I only have a little experience with TypeScript, and I want to make sure I resolve this issue correctly, even if it just means adding a section to the README with some guidance on using this library with TypeScript.

@Beraliv
Copy link

Beraliv commented May 23, 2021

@simonplend

Why does making object a string literal fix things?

I checked the validate function implementation and found string literals there:

  1. Open https://github.com/simonplend/express-json-validator-middleware/blob/main/src/index.d.ts#L27
  2. It uses ValidatorFunction – https://github.com/simonplend/express-json-validator-middleware/blob/main/src/index.d.ts#L18-L20
  3. Inside it uses AllowedSchemahttps://github.com/simonplend/express-json-validator-middleware/blob/main/src/index.d.ts#L13-L16
  4. Inside JSONSchema7 you will find type – https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/json-schema/index.d.ts#L627
  5. This type should be a string literal – https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/json-schema/index.d.ts#L570-L577

Check out the example here: https://tsplay.dev/wee2Ew

  1. If you inline the code, it will infer string as a string literal (and you will have no error)
  2. If you use as const either for object or for a string, it will convert the string to string literal and you will have no error too

You need as const because of the way how you declared the type:

type ValidateParameters = { type: 'object', parameters: Record<string, unknown> } | { type: 'string'; parameters: string; };

If you use 'object' in type or interface, it means you use string literals and you require to pass it from outside the same way (as string literal)

Is it preferable to put as const after the object string, or after the whole object itself?

It you call as const on the whole object, all strings inside will become string literals. But if you have at least one string literal inside you schema (e.g. JSONSchema7), as const for the whole object will be helpful and easy way to fix the problem:

  1. If you have string in types, both string literal and string can be passed
  2. If you have string literal in types, only this string literal can be passed

You can check examples here: https://tsplay.dev/wj53YW

@simonplend
Copy link
Collaborator

@Beraliv Thank you so much for this detailed detective work, the thorough explanation, and for all of the examples too. They are really helpful.

I think I understand everything you've explained, but I'm going to read it all again with fresh eyes tomorrow.

I'm planning to add a short section to the project README with tips on usage with TypeScript. If you are happy to, I would really appreciate you looking over these documentation changes before I merge them. Would you mind me tagging you in the PR?

@Beraliv
Copy link

Beraliv commented May 23, 2021

Would you mind me tagging you in the PR?

Not at all, I'm fine with it 👍

@Beraliv
Copy link

Beraliv commented Jun 6, 2021

@simonplend do you still need help?

simonplend added a commit that referenced this issue Aug 3, 2021
When defining JSON schemas in TypeScript, you will receive a
compilation error unless you cast the schema object with `as const`.

Related issue: #39
@simonplend
Copy link
Collaborator

@Beraliv I've added a section to the README explaining how to define schemas in TypeScript. If you're still happy to, I'd be very grateful if you could review these changes for accuracy: #89

@simonplend
Copy link
Collaborator

I was looking at the documentation for the json-schema-to-ts package recently. I noticed their documentation also mentions the need to use as const when defining JSON schemas in TypeScript: https://www.npmjs.com/package/json-schema-to-ts#fromschema

@Beraliv
Copy link

Beraliv commented Aug 3, 2021

@Beraliv I've added a section to the README explaining how to define schemas in TypeScript. If you're still happy to, I'd be very grateful if you could review these changes for accuracy: #89

Sure, I can check it tomorrow

@simonplend
Copy link
Collaborator

I'm planning to merge #89 tomorrow so that I can close this issue and get v2.2.0 released.

Thanks for your help everyone!

simonplend added a commit that referenced this issue Aug 10, 2021
When defining JSON schemas in TypeScript, you will receive a
compilation error unless you cast the schema object with `as const`.

Related issue: #39
@alexb-uk
Copy link

alexb-uk commented Oct 8, 2021

Adding as const did not fix the errors for me.

My error:

Error:(30, 14) TS2322: Type '{ type: string; properties: { mustHaveField: { type: string; }; }; required: string[]; }' is not assignable to type 'ValidateFunction'.
  Type '{ type: string; properties: { mustHaveField: { type: string; }; }; required: string[]; }' is not assignable to type 'JSONSchema7'.
    Types of property 'type' are incompatible.
      Type 'string' is not assignable to type 'JSONSchema7TypeName | JSONSchema7TypeName[]'.

Solution:

const schema: JSONSchema7 = {
  type: 'object',
  properties: {
    mustHaveField: { type: 'boolean' },
  },
  required: ['mustHaveField'],
};

Also updated to the latest json-schema type definition to avoid type mismatch error. At time of writing that's "@types/json-schema": "^7.0.9",

HTH

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

Successfully merging a pull request may close this issue.

9 participants