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.

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

Jump to bottom

Args is type any in latest version (1.0.18) #63

Closed
cmdcolin opened this issue May 29, 2023 · 23 comments
Closed

Args is type any in latest version (1.0.18) #63

cmdcolin opened this issue May 29, 2023 · 23 comments

Comments

@cmdcolin
Copy link

cmdcolin commented May 29, 2023 •
edited

See codesandbox https://codesandbox.io/s/pedantic-violet-f0x5uf?file=/src/App.tsx

Edit: screenshot wasn't properly capturing the tooltip like I thought it was...so removed screenshot, but can hover over the height/width in the sandbox to see the type is any
@bvaughn
Copy link
Owner

bvaughn commented May 29, 2023

That's interesting. The types are good enough to warn about a mismatch if you add e.g. disableHeight={true} but they're reporting any for the values 🤔

@bvaughn
Copy link
Owner

bvaughn commented May 29, 2023

If I pass children as an explicit prop it seems to work
Screen Shot 2023-05-29 at 4 05 39 PM

@bvaughn
Copy link
Owner

bvaughn commented May 29, 2023

I'm not sure why TypeScript is distinguishing between a function passed as JSX child vs a children prop.

Here's how I'm testing the new type structure though:
https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbwFBzgYQuCA7AptmAGhTgGMpcBDGXAUQBtcR8iSAJAFQFkAZAQRgwowAEYBXGgGdiqAEpVSMAHIQAJrmIBfOADMomOACIKlRYYDcSJLgAekWHBgBPMLjhtowAF44YlegDK3m4AvIgkAO7AqjAAFgBccNhiICK4UJaokqT+uKoA6tFxicmp6Zaalrb28M6ucABq6TDAOYHBcGHIqLG4wADmsTAlKWkZJNm5qmx9g8NJo+VIldZ20LUubkFeoe6ePgT+224AZI3NrUfBlkh1bgBClJK4AAr6YJKd4ajYOKS4AH5EpIhMBsP1Mo5KP0lJRmEC4CDhOCKnAzgB5EDAGAAHk4vAEoPEUjx3B4ABFgAA3BhMFgAPkIRlIsWA9FUFGwhjgAB8jDh5JJgoZ6Tc7u5ZkN0dh6E43hAPl9Hs95Yqzt0yKz2ZzEgAKIU7RJNWCXdo7ACUnXpcHkpmUalwkPUOkoYnoMBmAyGCNKYydwEklBEjEKMQSjigYkdJAFuANgL18aNFzax0tIWtVIg0QqYs2cFDcWlstVnzCyte70+6pILLZHPwieCiQ8wgOfjNuHT1ttihU6iduBdbpghdiPsW41QqgDQcYnrmiSEUchsfjCP1zb2bd8VwtVrgWZzyzz9QXQz42AKRVipaVT0rCur3019Z1cE3hrgaYPvftA5IZ1XXdc8YAnMopzgICRzHcC-UA2dg1wUCERdehnn9QMkNgxI0IwmNsEFYINyTb9gm7Q9s1UXMrGqdZHHzO8wlA4s5SrXkCxvVi7z5UDL2vMNS1PLY-BoL4NV6L15l9JYsjaPJQJGCDIUmRgBOKBZlMiG8lPglZVhqMh6CeT4+AkCBjigOBbBoK9PgwLA8AIHFSyZAJRNwa0NU5dQoF1S0NVQCgYDEKBsAWeh6EhTRlisAB6OK4A4OMYEkKxyCoGhaWYAhdTMmALOCKAmQ1OttUbD9SJ-DMIvoGQoKHYCPUleYAEYACYAGYmTgZZzUsDLqDoRgcpgPLzMskray1BtsCbL9W28XdOwo5JIvqmcsPnFql0jDQ+oGkwspGlhxoKyaXzK2b5twZMTVTciDzWuqEK23Ax12qMtH6pBBuOulcvywqdmKy6ZvfT9bu3JbDhWp63Re6dEJDHTdH8Z56rXLdIbuloHv3GqEBizQfr+4aAbGoGLtK8GKpx857r3Lt4fW1652Qna0fQ-bUCxr96cW9smYoomDvSo7ydGs7gfSKbUASuAAAFUoAWmqXBFDVqB9Cs6a3zphA4EkuYmSiMM4BJlnEag5GOakz79pJm4yey06qaKuW4AV5XJDVuwNZgLWdd61Arohw3jaGU2bwt1aEY222PojL6DqAA

@bvaughn
Copy link
Owner

bvaughn commented May 29, 2023

Hm, I think I figured it out: https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAJQKYEMDGMA0cDeAoOOACQBUBZAGQEEYYpgAjAVxiQGdMDFUMA5CACZJOAXzgAzKBBBwARFB4xZAbjx4kAD0iw4MAJ5gkxaMABeEAHYwUAGwDKZowF5cXAO7ABMABYAuOBZMIAxIUKoiqpra8PqGcABqoTDAaLYOps6uhN5IwADm3jD+gcGh4ZFa0DEGRumZRCbmVmmOcABkCUkpLRmqeLFGRLkFMADyFjZ6AApSYGxwLviEaN7ANgIKFv4AFGyO-omw3faOAJQLAHzc6DD8QqqEAsBsKAw2SEP5hQD8-uK2bCQDzgTxebyQAHVPD5-HQmEC8BE1AM4FCvN5xpMZhA5gssnAVmsNkgtnBdvtjPQmtYThlzk4rsgbncEY9nq93p8RrCoPDgaCOZDod5fhIAQikf0asRhoUqBYBGifNjcYsuIT1psdnsMv46vTGYoWfz2eCuT8-uKTWD3kqRZabIDymoolVdNKVfMXOaxhNprN5gAfVHCzH+nFBmVfGDyxXCz19cRMCwYYCWOBUFgQOpQbZLAmrTUkzhsm0fWVYLgC8F20T+T3nfPAcRk6uciuNriEBQwJhQCwFombPNwDzo-wAJjgInOKHmKAsemBYiQjqMzdbpttws7hG7SF7-cHRYsI5y0f8AEZp7P54vl3BV4D8fvDwONcTTzg4OfuXBL9gY4wnAU4znAc7gfeXAiIiagAPRwXAJDsDAbBqNsXAADyZjA2aOFAx6fk4OBnhWgHCjelxkl2cCYU8ABucBsPo7zEd+v6FOR6LTmIcEXFwpy8fxpyqBhhDYVmOaEZsxEjkB3iUQy1F7rRDFMSxSBsaOFEiEJAliG25bRsRcJIHpIl4GJtE4XhGQER+MkkexFaKVcVniWpzF6KxODOdGPFwHx+kgluQroiZvJmYFwmiVwCFwAAAqhAC0URIBgqVQFIUBYTZUkOSSsl+SMXE+K5yl7nRwCMV5PnFZx2ncbp0XBYZPoRfC5mxYQ8VJWwqVaOlMCZdluWSfh0mFU5P5kY1ZVgUp7mqdV6neZpvkzdGpUKc1QWEIJIVlnaHVRUFFlAA

@bvaughn
Copy link
Owner

bvaughn commented May 29, 2023

Fixed in 63c6c8e. Will release in a bit.

@bvaughn bvaughn closed this as completed May 29, 2023
@V-Mann-Nick
Copy link

I still have the same problem with 1.0.19. Types are any if passed as jsx child. Passing explicitly as children prop works though.

https://codesandbox.io/s/react-virtualized-auto-sizer-any-type-ny0l6l

@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023

Maybe there's an issue with the TypeScript version being used on Code Sandbox. I'm not sure.

TypeScript properly handles the types for the latest release, so I don't know what I can further do about this:
https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgIilQ3wG4AoUSWOAQQFcYIBlYALySjm1wKPRgBaAG7BY9FABt2SACaCUjCIIDOMqGXLkA9NrgAVJCpgqtACnJw4AHgZNWHLmgAWwSbKIA7ALwBvM75wzkjAAObOMAA0cADuwLIwznAAvgCUcN4AfHAWVlbWssDCcMYAnpJIfoHBYRHRcQlJyclw2pmWcKktbeSpFLk2dizqcC5uHkg+-oENiSnpWTkd+YXFZRVVsfFzza3tVl1whSooAEYVABIh4TB+MFD0SN3tfeQDtkoOnKOu7l5+ASC1wi8wy2QGKyKJRg5UqvmqwPgux6BxaxzOFQA6ttnHcHk89r1+h1dHAAAImQRIAAeYCQGCpUBwUA6H3sIzGf0mAIRtSiW0aoMWEJsq2hsM2NRu9RxKWeHUO6POSCufLxj3lrwGpIpKiptPpQk4zNZQy+Tl+EymgKldQFOwW4OWoqh6zhvOl9qa8tRR2AJ2V2Ma6oJPVeQA

@V-Mann-Nick
Copy link

I'm using the latest version of typescript in my editor (5.0.4) and I have the same issue there.

@Andarist
Copy link
Contributor

@V-Mann-Nick could u prepare a repository that would showcase the problem?

@V-Mann-Nick
Copy link

V-Mann-Nick commented May 30, 2023 •
edited

Yeah sure. Here it is: https://github.com/V-Mann-Nick/react-virtualized-auto-sizer-typing-bug

git clone https://github.com/V-Mann-Nick/react-virtualized-auto-sizer-typing-bug.git
cd react-virtualized-auto-sizer-typing-bug
pnpm install
pnpm tsc

@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023 •
edited

FWIW opening that repo locally picks up the correct types for me.
Screen Shot 2023-05-30 at 9 31 30 AM

@Andarist
Copy link
Contributor

The problem is likely related to contextual types for children used through the JSX children (and not through the explicit children prop on the component). We can see that it fails in 5.0.4 in this TS playground but it already works in the latest nightlies of TypeScript, see the playground here. I think that this has been fixed by one of my TypeScript PRs (see this one).

Since this was a bug in TypeScript itself... I'm not sure if I can figure out a library-land fix for this but gonna take another look at this some time later today.

On the flip side... this should start working in TS 5.1 which is supposed to get released... today 😅 (according to the timeline posted here)

@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023

Thanks Andarist!

@V-Mann-Nick
Copy link

Ok. I guess I'll just use the children prop for now and look at it again when TS 5.1 released. Perhaps worth reopening this issue until it has been resolved?

Thanks for the quick responses 👍

@Andarist
Copy link
Contributor

Andarist commented May 30, 2023 •
edited

@bvaughn I think that somehow you are using a different TS version locally, ain't sure how though. Could you recheck with cmd+P: "Select TypeScript Version..."? I tested this repro and can observe the problem as expected

I tried a couple of things locally (different variations based on height?: never as similar) and can't make it work with TS 5.0. So if you intend to keep this shape of the API, I would advise just waiting for the 5.1 release which should come really soon.

@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023

Seems like I'm using a pre-release TS version (5.2 dev from a few days ago). Don't know why.

I'm sorry for the confusion this is causing folks, but I don't want to spend time reverting this change (the past couple of releases really) because of a TypeScript issue if it's being fixed/released today. This is a side side project for me.

@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023

To be clear, there is no "fix" really. The previous types were inaccurate. (They didn't have this "any" type issue, but they did incorrect report height/width when one had been disabled.)

I don't know of a good way type this that satisfies TypeScript, at least not without rewriting this component to be a function component (which may help but would also be a breaking change and raise the min React version).

@shiftyp
Copy link

shiftyp commented May 30, 2023

The issue is in the TS version. It's this issue, which exits in Typescript v4 and is fixed in v5:

microsoft/TypeScript#52111

shiftyp added a commit to shiftyp/react-virtualized-auto-sizer that referenced this issue May 30, 2023
@shiftyp
Update typescript required version to ^5.0.4 to fix bvaughn#63
ea07dc8 
Fixes bvaughn#63
@shiftyp shiftyp mentioned this issue May 30, 2023
Closed
@bvaughn
Copy link
Owner

bvaughn commented May 30, 2023

The issue is in the TS version. It's this issue, which exits in Typescript v4 and is fixed in v5:

@shiftyp Interesting. Why is @V-Mann-Nick seeing this problem with TypeScript 5.0.4?

@shiftyp
Copy link

shiftyp commented May 30, 2023 via email

@rentalhost
Copy link

I am using TS 5.1.3 and auto-sizer 1.0.20 and the problem stills happen. :(

@bvaughn
Copy link
Owner

bvaughn commented Jun 3, 2023

You will either need to upgrade Typescript to the latest (5.1) or pass children as a. Explicit prop rather than JSX children. I can’t help/fix this otherwise.

@krailler
Copy link

krailler commented Aug 25, 2023 •
edited

Try this ({ width, height }: VerticalSize): ReactElement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@bvaughn @shiftyp @rentalhost @krailler @cmdcolin @Andarist @V-Mann-Nick