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

Got error TS2315 with recently released TS 3.9 #24

Closed
Gounlaf opened this issue May 13, 2020 · 17 comments
Closed

Got error TS2315 with recently released TS 3.9 #24

Gounlaf opened this issue May 13, 2020 · 17 comments
Assignees
Labels

Comments

@Gounlaf
Copy link

Gounlaf commented May 13, 2020

Hi,

I was using your great package on TS 3.8
I've just installed TS 3.9; since, the compiler complains about (mainly, but not only) this error:

error TS2315: Type 'Static' is not generic.

It might be due to this BC but i'm not sure: Type Parameters That Extend any No Longer Act as any

Right now I will stay on TS 3.8.

I don't know how I can help you more on this issue (except submitting a PR, but I don't know how to fix the issue right now)

Thanks,
Regards.

@apyrkh
Copy link

apyrkh commented May 13, 2020

Yes, the same for me :(

@sinclairzx81
Copy link
Owner

@Gounlaf Hi, Thanks for letting me know. Just had a look and yes indeed, that is quite the extreme level of breakage. I'll do some reading up on these changes and see what can be done to resolve.

Many Thanks

@fox1t
Copy link
Contributor

fox1t commented May 14, 2020

I was to report the same. All of the projects just stopped building. You can find some reference here microsoft/TypeScript#38460 and here microsoft/TypeScript#38571
Let me know if you need help!

@sinclairzx81
Copy link
Owner

Thanks @fox1t

Yeah, its quite the regression. Had a look early on today, but havent fully grasped the implications of the 3.9 changes. Would certainly be open to any and all assistence !!!

I dont antipicipate im going to be able to get around to resolving this within the next few days at least. I'd certainly be willing to accept any PRs in the interim however.

Apologies for the inconvenience. Will keep an eye on those issues youve linked. Thanks again.

@fox1t
Copy link
Contributor

fox1t commented May 14, 2020

It seems that is side effect of this behavior microsoft/TypeScript#37195
some more context: microsoft/TypeScript#38542

sinclairzx81 added a commit that referenced this issue May 15, 2020
@sinclairzx81
Copy link
Owner

sinclairzx81 commented May 15, 2020

@Gounlaf @apyrkh @fox1t

Just published a fix that should resolve the build errors on TS 3.9.2. Updates are published on @sinclair/[email protected].

The issues were around trying to flatten readonly, optional modifier objects into a presentable object for intellisense. This has been removed for the time being, the types should still work as to spec.

const T = Type.Object({
    a: Type.ReadonlyOptional(Type.String()),
    b: Type.Readonly(Type.String()),
    c: Type.Optional(Type.String()),
    d: Type.String()
})
type S = Static<typeof T>

Pre 3.9.2

type S = {
    readonly a?: string
    readonly b: string
    c?: string
    d: string
}

Post 3.9.2

type S = {
    readonly a?: string;
} & {
    readonly b: string;
} & {
    c?: string;
} & {
    d: string;
}

If you're able to test your side, that would be great.

@sinclairzx81 sinclairzx81 self-assigned this May 15, 2020
@apyrkh
Copy link

apyrkh commented May 15, 2020

@sinclairzx81
Thank you!!
The initial problem has gone but now there is one more:

import { Type } from '@sinclair/typebox';
import fastJson from 'fast-json-stringify';

enum Enum {
  A = 'A',
  B = 'B',
  C = 'C',
}
const schema = Type.Object({
  param1: Type.String(),
  enum: Type.Enum(Enum, { type: 'string' }),
});

const stringify = fastJson(schema);  // <=== this string doesn't pass ts type validation
stringify({});

I don't know whether the issue in fast-json-stringify or in your package

@apyrkh
Copy link

apyrkh commented May 15, 2020

I found out a workaround how to define Enum:

const schema = Type.Object({
  param1: Type.String(),
  enum: {
    enum: Object.values(Enum),
    type: 'string',
  },
});

@sinclairzx81
Copy link
Owner

@apyrkh Glad the updates resolved those TS 3.9.2 issues :)

As for the fast-json-stringify, had a quick look, and it may be that fast-json-stringify is being too strict on the type property for enum which I believe is optional to support varying enum value types. TypeBox currently does not emit a type property for enum schemas as TS enums values may either be number | string (tho it is expected its going to be one or the other).

The current definition of TEnum is as follows.

export type TEnum<T extends string | number> = { enum: Array<T> } & UserDefinedOptions

With reference to.

https://json-schema.org/understanding-json-schema/reference/generic.html#enumerated-values

In all, a change to TypeBox enums are a bit tangential to this particular issue. If you're ok with these updates to resolve this TS 3.9.2 issue, I would be pretty keen to close off this one and happy to review a possible enum type change in a separate GH issue thread.

Cheers
S

@Gounlaf
Copy link
Author

Gounlaf commented May 15, 2020

Error TS2315 disappear :)
Now I have issue with TS2589 (Type instantiation is excessively deep and possibly infinite). I have to figure out why =/

@Gounlaf
Copy link
Author

Gounlaf commented May 15, 2020

Don't know it's related to typebox or not;

const FooTypeSchema =  Type.Object({
  a: Type.String(),
  b: Type.Boolean(),
})

export type FooType = Static<typeof FooTypeSchema>

// This one is defined manually without schema
export type BarType = {
  c: string;
  d: boolean;
}


export function convertFoo(foo: FooType): BarType {
  return {
    c: foo.a,
    d: foo.b
  }
}

// this one triggers TS2589
export function buggyConvertFoos(...foos: FooType[]): BarType[] {
  return foos.map(foo => convertFoo(foo))
}


// this one doesn't
export function convertFoos(...foos: FooType[]): BarType[] {
  const map: BarType[] = []

  foos.forEach(foo => map.push(convertFoo(foo)))

  return map
}

I don't know how to find if it's related to typebox or not.
Any idea?

@sinclairzx81
Copy link
Owner

sinclairzx81 commented May 15, 2020

@Gounlaf Thanks for the repro.

At this stage, I really have no idea if this is a TypeBox issue or a TypeScript regression issue. But both these issues seem tied to recent updates related to TS intersection types and I am leaning somewhat to this being a TypeScript optimisation problem (where TypeBox is hitting some recursion limit)

I have been able to narrow down the issue however; it relates to how TypeBox decomposes object properties for their optional and readonly modifiers which are then recomposed via an intersection. TypeBox provides 3 modifier types ...Type.Optional(..), Type.Readonly(..) and Type.ReadonlyOptional(..).

I can push a fix which resolves TS2589, however it does involve removing the Type.ReadonlyOptional(...) modifier. Offending code below.

// Extract 'optional', 'readonly' and 'default' property keys from T
type ReadonlyOptionalPropertyKeys<T> = { [K in keyof T]: T[K] extends TReadonlyOptional<infer U> ? K : never }[keyof T]
type ReadonlyPropertyKeys<T> = { [K in keyof T]: T[K] extends TReadonly<infer U> ? K : never }[keyof T]
type OptionalPropertyKeys<T> = { [K in keyof T]: T[K] extends TOptional<infer U> ? K : never }[keyof T]
type PropertyKeys<T> = keyof Omit<T, OptionalPropertyKeys<T> | ReadonlyPropertyKeys<T>>

type StaticObjectProperties<T> =
  // commenting the line below resolves TS2589
  // { readonly [K in ReadonlyOptionalPropertyKeys<T>]?: Static<T[K]> } &
  { readonly [K in ReadonlyPropertyKeys<T>]: Static<T[K]> } &
  { [K in OptionalPropertyKeys<T>]?: Static<T[K]> } &
  { [K in PropertyKeys<T>]: Static<T[K]> }

At this stage, I'm unable to produce a trivial repro for the issue to submit to the TS team, and I am a bit uneasy about hastily changing the API (potentially removing Type.ReadonlyOptional(..)) if the issue actually rests with TypeScript.

Give me a day or so to have a think about next steps. Any feedback or insights into the TS 3.9 update happening elsewhere in the community would be helpful in understanding the extent of the problem. In the interim, and if possible, I would recommend keeping with TS 3.8.

@sinclairzx81
Copy link
Owner

@Gounlaf

Have decided to just drop Type.ReadonlyOptional(..) which should resolve that unusual TS2589 error. I have done a minor semver revision, bumping TypeBox to 0.10.0.

Are you able to confirm things are fine yourside? If so, would be good to close off this issue.

@Gounlaf
Copy link
Author

Gounlaf commented May 20, 2020

Hi @sinclairzx81,

Sorry, I still not have tested your latest version.
I will soon.

@Gounlaf
Copy link
Author

Gounlaf commented May 23, 2020

Hi @sinclairzx81,

On TS 3.9.3, using your latest version, TS2315 & TS2589 are no more triggered.
I can use again buggyConvertFoos too.

I've removed my Enums definitions 'cause I don't need them anymore, so I didn't check if there is still issue about it or not.

@sinclairzx81
Copy link
Owner

@Gounlaf Alright cool, thanks for checking your side, will close this one off.

Cheers
S

@njavilas2015
Copy link

nights I come for a similar good topic in another library and I discovered that if we export the interface with export type { } it works

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

No branches or pull requests

5 participants