Skip to content

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Mar 23, 2023

Fixes #53459
Reopens #53255

My fix for #53255 wasn't fully correct; I didn't test method implementation / the case where there's already an effective rest type.

For now, I think I'd rather just revert my fix and later try and "really" fix this via #53398.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 23, 2023
@jakebailey
Copy link
Member Author

@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 23, 2023

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 604e355. You can monitor the build here.

Update: The results are in!

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Alright, guess we're looking towards the fully generic solution, then.

@jakebailey
Copy link
Member Author

jakebailey commented Mar 23, 2023

Yeah :(

I have it half-working; it turns out that we already have a helper that mostly does what I want: getMutableArrayOrTupleType

But, I think I have to apply it in some places but not others, and the more I think about how it's supposed to work, the more edge cases I see. For example, if a function is <T>(...args: T) => T to return args, is that supposed to be mutable, as that's the more "useful" type?

Also, { "0": string } which I guess is a tuple isn't handled.

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.
There were interesting changes:

Main only errors:

Package: ckeditor__ckeditor5-upload
Error:

Error: /home/vsts/work/1/s/DefinitelyTyped-tools/packages/dtslint-runner/DefinitelyTyped/types/ckeditor__ckeditor5-upload/src/filereader.d.ts:30:5
ERROR: 30:5  expect  TypeScript@local compile error: 
Property 'set' in type 'FileReader' is not assignable to the same property in base type 'Observable'.
  Type '{ (option: Record<string, unknown>): void; (name: string, value: unknown): void; }' is not assignable to type '(...args: [option: Record<string, unknown>] | [name: string, value: unknown] | [name: string]) => void'.
    Types of parameters 'option' and 'args' are incompatible.
      Type '[option: Record<string, unknown>] | [name: string, value: unknown] | [name: string]' is not assignable to type 'readonly [option: Record<string, unknown>]'.
        Type '[name: string, value: unknown]' is not assignable to type 'readonly [option: Record<string, unknown>]'.
          Source has 2 element(s) but target allows only 1.
ERROR: 31:5  expect  TypeScript@local compile error: 
Property 'set' in type 'FileReader' is not assignable to the same property in base type 'Observable'.
  Type '{ (option: Record<string, unknown>): void; (name: string, value: unknown): void; }' is not assignable to type '(...args: [option: Record<string, unknown>] | [name: string, value: unknown] | [name: string]) => void'.

    at testTypesVersion (/home/vsts/work/1/s/DefinitelyTyped-tools/packages/dtslint-runner/node_modules/@definitelytyped/dtslint/dist/index.js:194:15)
    at async runTests (/home/vsts/work/1/s/DefinitelyTyped-tools/packages/dtslint-runner/node_modules/@definitelytyped/dtslint/dist/index.js:151:9)

You can check the log here.

@jakebailey jakebailey merged commit 9bd1a32 into microsoft:main Mar 23, 2023
@jakebailey jakebailey deleted the fix-53459 branch March 23, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#53258 breaks ckeditor__ckeditor5-upload on DT
3 participants