fastify / env-schema

Validate your env variable using Ajv and dotenv
MIT License
212 stars 25 forks source link

Use with fluent-json-schema now requires to call valueOf in ObjectSchema when using Typescript #138

Open Ceres6 opened 1 year ago

Ceres6 commented 1 year ago

Prerequisites

Last working version

5.1.0

Stopped working in version

5.1.1

Node.js version

18.x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

12.3

💥 Regression Report

Typings in Typescript now throw an error when using a fluent-json-schema generated schema as input for envSchema

Steps to Reproduce

In a Typescript file the next block of code reproduces the issue

import envSchema from 'env-schema'
import S from 'fluent-json-schema'

envSchema({
  schema: S.object()
})
Type 'ObjectSchema' is not assignable to type 'AnySchema | UncheckedJSONSchemaType<EnvSchemaData, false> | undefined'.
  Type 'ObjectSchema' is not assignable to type '{ type: "object"; additionalProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; unevaluatedProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; ... 7 more ...; maxProperties?: number | undefined; } & { ...; } & { ...; } & { ...; }'.
    Property 'type' is missing in type 'ObjectSchema' but required in type '{ type: "object"; additionalProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; unevaluatedProperties?: boolean | UncheckedJSONSchemaType<unknown, false> | undefined; ... 7 more ...; maxProperties?: number | undefined; }'.ts(2322)

Nevertheless the following block of code does not throw an error

import envSchema from 'env-schema'
import S from 'fluent-json-schema'

envSchema({
  schema: S.object().valueOf
})

Expected Behavior

As ObjectSchema from fluent-json-schema was removed from typings in #137 it might be solved otherwise or maybe be documented to avoid future confusion

climba03003 commented 1 year ago

cc @klaseca

I think I would revert all those changes. It cannot support all the third-party library. Especially, it cannot support fluent-json-schema.

klaseca commented 1 year ago

Under the hood, valueOf is called when passing a schema from fluent-json-schema. How about we remove this code and explicitly call valueOf when passing the schema? It seems to me that if env-schema should work not only with fluent-json-schema, then there should be no code specifically for this library

climba03003 commented 1 year ago

How about we remove this code and explicitly call valueOf when passing the schema?

No, it should support out-of-the-box. Since, fastify itself do the same. cc @fastify/core WDYT?

Anyway, it would be a breaking change in either end. If no other fixes come up, the only choice is revert on current release.

Uzlopak commented 1 year ago

I would also revert the problematic PR and maybe add fluent-json-schema as dependency or as peerDependency

klaseca commented 1 year ago

Perhaps the best solution in this situation would be to return the object type for the schema. At the same time, leave the re-import of the JSONSchemaType type from the ajv package. In this case, we will return the previous fully working state and leave the option to declare a type-checked schema

mcollina commented 1 year ago

Fastify can support fluent-json-schema without the need of it being a dependency. Take a look how it's done there.

jessekrubin commented 1 year ago

FWIW: Something similar is going on with types here when using with typebox