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

Type error with Typescript 4.8 #8465

Closed
fcollonval opened this issue Oct 10, 2022 · 4 comments · Fixed by #8473
Closed

Type error with Typescript 4.8 #8465

fcollonval opened this issue Oct 10, 2022 · 4 comments · Fixed by #8473
Labels
Need Clarification ❔ Needs clarification before we can proceed.

Comments

@fcollonval
Copy link
Contributor

The following template parameter is throwing an error with TypeScript 4.8 because the template of Conditional type is restricted to a subset Conditional<CD extends FieldDef<any> | DatumDef | ValueDef<any> | ExprRef | SignalRef>

export function isConditionalParameter<T>(c: Conditional<T>): c is ConditionalParameter<T> {

The error raised is

  ../node_modules/vega-lite/build/src/channeldef.d.ts(63,66): error TS2344: Type 'T' does not satisfy the constraint 'SignalRef | ExprRef | FieldDef<any, any> | DatumDef<string, SignalRef | DateTime | ExprRef | PrimitiveValue> | ValueDef<...>'.
    Type 'T' is not assignable to type 'DatumDef<string, SignalRef | DateTime | ExprRef | PrimitiveValue>'.

Seen in https://github.com/jupyterlab/jupyterlab/actions/runs/3217544851/jobs/5260639447

@domoritz
Copy link
Member

Is this the same as #8454?

@fcollonval
Copy link
Contributor Author

Yes it is - but using compiler option is not an option. The code really requires the fixed mentioned in #8454 log:

63 export declare function isConditionalParameter<T>(c: Conditional<T>): c is ConditionalParameter<T>;
                                                  ~
This type parameter might need an `extends SignalRef | ExprRef | FieldDef<any, any> | DatumDef<string, SignalRef | DateTime | ExprRef | PrimitiveValue> | ValueDef<...>` constraint.

The reason is the type constraint on Conditional<T extends ...> and ConditionalParameter<T extends ...>. So the type parameter in isConditionalParameter function must extend it too.

@domoritz
Copy link
Member

I see. If skipLibCheck is not an option for you, do you have cycles to help with fixing the types in Vega-Lite? Does it make sense to track this under #5477 instead of this separate issue?

@domoritz domoritz added the Need Clarification ❔ Needs clarification before we can proceed. label Oct 10, 2022
@fcollonval
Copy link
Contributor Author

Sure I can open a PR. But I would advice keeping it separated from #5477.

fcollonval added a commit to fcollonval/vega-lite that referenced this issue Oct 11, 2022
domoritz pushed a commit that referenced this issue Dec 18, 2022
* Constrain `isConditionalParameter` type parameter

Fixes #8465

* Lint the code
BradyJ27 pushed a commit to BradyJ27/vega-lite that referenced this issue Oct 19, 2023
* Constrain `isConditionalParameter` type parameter

Fixes vega#8465

* Lint the code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need Clarification ❔ Needs clarification before we can proceed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants