Skip to content

fix(types): support fluent-json-schema #244

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.

Sign up for GitHub

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

Open
salluexez wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(types): support fluent-json-schema #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ interface DotenvOptions {

type EnvSchema = typeof envSchema

type FluentSchema = {

@ilteoood ilteoood Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • why it needs to change: fluent-json-schema's own README documents isFluentSchema as the public detection flag; isFluentJSONSchema is also set on every BaseSchema<T> but is not the documented public contract. The two-field structural type is broader than what the upstream docs promise to set.
  • code suggestion: 
    type FluentSchema = { isFluentSchema: boolean }
  • why the suggestion differs: narrows the structural requirement to the single documented public flag; an object with isFluentSchema: boolean is a fluent schema per the upstream README, and the second field does not add coverage the documented contract guarantees.

isFluentSchema: boolean;

@Puppo Puppo Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @salluexez, thanks for your PR.
I'm not sure about this.
It seems like a Fluent workaround, not really an issue with this package.
In this branch, https://github.com/Puppo/env-schema/tree/test-fluent-schema, I prove that we should reach the same result without touching the codebase.

I'm happy to let you review your pr to add these tests if you'd like.
If you proceed, please also remember to add the test example to the JavaScript tests.

isFluentJSONSchema: boolean;
}

declare namespace envSchema {
export type { JSONSchemaType }

Expand All @@ -25,7 +30,7 @@ declare namespace envSchema {
}

export type EnvSchemaOpt<T = EnvSchemaData> = {
schema?: JSONSchemaType<T> | AnySchema;
schema?: JSONSchemaType<T> | AnySchema | FluentSchema;

@ilteoood ilteoood Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • why it needs to change: this PR changes the types only. The runtime detection in index.js still uses Symbol.for('fluent-schema-object') + .valueOf(). The structural type is a type-level approximation of what the runtime already accepts, not a new contract, and a future reader of this line could infer that adding a field here changes runtime behavior.
  • code suggestion: add a short // type-only: runtime uses Symbol.for('fluent-schema-object') in index.js above the union, or attach a JSDoc to the FluentSchema type pointing at the runtime path so the two stay in sync.
  • why the suggestion differs: makes the type-only nature explicit at the point of use; prevents a future contributor from "tightening" the type to match a runtime field they think is checked and breaking valid fluent schemas.

data?: [EnvSchemaData, ...EnvSchemaData[]] | EnvSchemaData;
env?: boolean;
dotenv?: boolean | DotenvOptions;
Expand Down
14 changes: 14 additions & 0 deletions types/index.tst.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import envSchema, {
default as envSchemaDefault,
} from '.'
import Ajv, { KeywordDefinition, JSONSchemaType } from 'ajv'
import S from 'fluent-json-schema'
import { Static, Type } from 'typebox'

interface EnvData {
Expand Down Expand Up @@ -46,6 +47,11 @@ const optWithSchemaTypebox: EnvSchemaOpt = {
}
expect(optWithSchemaTypebox).type.toBe<EnvSchemaOpt>()

const optWithFluentSchema: EnvSchemaOpt = {
schema: S.object().prop('PORT', S.number().default(3000).required()),
}
expect(optWithFluentSchema).type.toBe<EnvSchemaOpt>()
Comment on lines +50 to +53

@Puppo Puppo Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const optWithFluentSchema: EnvSchemaOpt = {
schema: S.object().prop('PORT', S.number().default(3000).required()),
}
expect(optWithFluentSchema).type.toBe<EnvSchemaOpt>()
const optWithFluentSchema = {
schema: S.object().prop('PORT', S.number().default(3000).required()),
}
expect(optWithFluentSchema).type.toBeAssignableTo<EnvSchemaOpt>()


const optWithSchemaWithType: EnvSchemaOpt<EnvData> = {
schema: schemaWithType,
}
Expand Down Expand Up @@ -113,5 +119,13 @@ expect(envSchemaWithType).type.toBe<EnvData>()
const envSchemaTypebox = envSchema<SchemaTypebox>({ schema: schemaTypebox })
expect(envSchemaTypebox).type.toBe<SchemaTypebox>()

const envSchemaFluent = envSchema<EnvData>({

@ilteoood ilteoood Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • why it needs to change: this assertion uses S.object<EnvData>() to force the generic T, but fluent-json-schema does not infer T from .prop(...) arguments (unlike TypeBox). Users will not naturally type fluent schemas this way, and the test reads as promising more type inference than fluent-json-schema delivers.
  • code suggestion: either drop this assertion (the optWithFluentSchema block at line 50 already proves the regression fix), or replace it with a .valueOf() assertion that locks in the documented escape hatch from Use with fluent-json-schema now requires to call valueOf in ObjectSchema when using Typescript #138, e.g. expect(S.object().valueOf as unknown as object).type.toBeAssignableTo<JSONSchemaType<EnvSchemaData>>(). If the generic-T path must be exercised, leave a one-line comment that it is a manual-typing witness, not runtime validation.
  • why the suggestion differs: removes the over-claim about fluent-json-schema's type inference; the replacement locks in the workaround the original issue explicitly mentioned, and the regression is still proven by the first assertion.

schema: S.object<EnvData>().prop(
'PORT',
S.number().default(3000).required()
),

@Puppo Puppo Oct 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
),
).valueOf(),

})
expect(envSchemaFluent).type.toBe<EnvData>()

expect(keywords.separator).type.toBe<KeywordDefinition>()
expect(envSchema.keywords.separator).type.toBe<KeywordDefinition>()