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

Add Schema type declaration #121

Open
oliverfindl opened this issue Dec 30, 2020 · 11 comments
Open

Add Schema type declaration #121

oliverfindl opened this issue Dec 30, 2020 · 11 comments

Comments

@oliverfindl
Copy link

  • Operating System: Ubuntu 16.04 LTS
  • Node Version: 14.15.3
  • NPM Version: 6.14.10
  • webpack Version: ^4
  • schema-utils Version: ^3

Feature Proposal

Please add standalone Schema type declaration.

Currently is possible to import it manually from validate method type declarations, but it is not convenient:

import * as schemaUtils from "schema-utils";
import { Schema } from "schema-utils/declarations/validate";

Feature Use Case

To use typed schema object, that is passed into validate method.

@alexander-akait
Copy link
Member

types auto generated by typescript

@oliverfindl
Copy link
Author

oliverfindl commented Dec 30, 2020

Hello,

I see. Are there any plans for standalone types support, e.g. using DefinitelyTyped same way as loader-utils?

My current project using schema-utils looks like:

import * as schemaUtils from "schema-utils";
import { Schema, ValidationErrorConfiguration as SchemaValidationErrorConfiguration } from "schema-utils/declarations/validate";

// copied from: https://github.com/webpack/schema-utils/blob/master/declarations/validate.d.ts#L35
// this could / should be defined as its own type and be imported as above and therefore preventing breaking changes in future
type SchemaOptions = Array<object> | object;

const OPTIONS_SCHEMA: Schema = { /* ... */ };

/* ... */

const options: SchemaOptions & LoaderOptions = Object.assign({}, DEFAULT_OPTIONS, loaderOptions);
const schemaConfiguration: SchemaValidationErrorConfiguration = { name: PACKAGE_NAME, baseDataPath: "options" };

schemaUtils.validate(OPTIONS_SCHEMA, options, schemaConfiguration);

Which is kinda cumberstone and imported / defined types are not namespaced in schemaUtils, so they are polluting global namespace.

Thanks.

@alexander-akait
Copy link
Member

Are there any plans for standalone types support, e.g. using DefinitelyTyped same way as loader-utils?

We already provide types

Which is kinda cumberstone and imported / defined types are not namespaced in schemaUtils, so they are polluting global namespace.

Where?

Using:

import { Schema, ValidationErrorConfiguration as SchemaValidationErrorConfiguration } from "schema-utils/declarations/validate";

is normal, it is part of public API

Again, we don't write types, it generates by typescript

@oliverfindl
Copy link
Author

Hello,

Are there any plans for standalone types support, e.g. using DefinitelyTyped same way as loader-utils?

We already provide types

Provided types are in different declaration file instead of index.d.ts which is loaded together with index.js. Therefore its required to import them manually, which pollutes global namespace (this can be kinda solved by prefixing / renaming them in import statement) instead of having them in one single variable.

Which is kinda cumberstone and imported / defined types are not namespaced in schemaUtils, so they are polluting global namespace.

Where?

In provided example.

Using:

import { Schema, ValidationErrorConfiguration as SchemaValidationErrorConfiguration } from "schema-utils/declarations/validate";

is normal, it is part of public API

Same response as first one.

Again, we don't write types, it generates by typescript

I understand and I don't want to argue, so if there are no plans for this, then feel free to close this issue.

Thanks.

@alexander-akait
Copy link
Member

Why do not install json-schema? WE don't know what is schema will be used, you can use 4/6/7/etc version, I think it is part of your code

@oliverfindl
Copy link
Author

Hello,

why use another package, if you already got defined possible Schema type in declarations/validate.d.ts#L11 (I understand its auto-generated by tsc and therefore you can't modify it). Not to mention, your Schema type might change in future and it won't be fully compatible anymore, so I prefer to have it included in same package. And as I iterated my code, types for rest of input arguments for validate method should be included too.

I'm not familiar with generating types from javascript files, therefore I could not suggest solution different than write declarations manually.

Thanks.

@alexander-akait
Copy link
Member

In theory we can export Schema, but yes, it can be changed, so it is why not exported

@oliverfindl
Copy link
Author

Hello,

this was my initial idea. If you include Schema type, it will be always up-to-date to used version of schema-utils. If I defined Schema type myself, in future it could be different to what schema-utils is expecting (although, not very likely to happen).

Cleanest solution from my perspective would be using same approach as loader-utils as I suggested here. Unfortunately, it requires write declarations manually.

We can let this issue open for now, so more devs can suggest different / better solution.

Thanks.

@oliverfindl oliverfindl changed the title Add Schema type declaration Add Schema type declaration Dec 31, 2020
@Tofandel
Copy link

Tofandel commented Dec 21, 2022

I didn't really understand this issue, but I think I'm having the same problem

Why is the schema 3 possible JSONSchema?
Schema = (JSONSchema4 | JSONSchema6 | JSONSchema7) & Extend;

The JSONSchema4 is outdated and will not trigger typescript errors if you create an invalid ajv schema

An example

const schema: Schema = {
  properties: {
    test: {
      type: "boolean",
      required: true, // This is a valid value according to JSONSchema4 but not JSONSchema6 and JSONSchema7
    }
  }
}

Which when validating it on runtime will trigger a schema error

Shouldn't we use the ajv JSONSchemaType exported type instead

@alexander-akait
Copy link
Member

Yeah, it was implemented in the such way because ajs had no types before, but a lot of things were changed, so PR welcome

@Tofandel
Copy link

I'll probably make a PR when ajv-validator/ajv#2180 is resolved currently TS is too strict and requires a complete rewrite of the schema which will turn it into a big breaking change

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

3 participants