-
Notifications
You must be signed in to change notification settings - Fork 673
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
Several TypeScript errors when upgrading to 0.16.1 #2456
Comments
Thanks for the issue, and the reproduction @matt-koevort. It's interesting, because the error shows up on a scale read from the theme, but not if it's a standalone variable of the same type. Looking into it! |
Okay, it seems that we can no longer read string keys from an union of array and object with string index signature. This is a bit of a tricky case then, because we need to keep accepting an array OR a dict in the theme user gives to the theme provider, but give out a broader type in I'm curious why does it work as expected in my commercial production project which is using the same versions. In the meantime, may I interest you in using /** @jsxImportSource theme-ui */
import { makeTheme } from "@theme-ui/css/utils";
const theme = makeTheme({
shadows: {
firstLevel: "0 0 4px 2px rgba(0, 0, 0, .125)",
},
});
type MyTheme = typeof theme;
function App() {
return (
<>
<div
sx={{
boxShadow: (theme) => (theme as MyTheme).shadows![0], // errors as expected
}}
/>
<div
sx={{
boxShadow: (theme) => (theme as MyTheme).shadows.firstLevel, // this works, and firstLevel gets autocompleted
}}
/>
</>
);
} |
@hasparus thanks for looking into this. To be honest I had a fiddle with this in TypeScript and also found it difficult to achieve the broader type as you mentioned. The only problem we would have with your suggestion, is that (a) we have far too many instances of the theme callback so updating all of them would be a bit of a headache, and (b) we don't actually have the specific typing on the theme as this is a library that is used in another application which actually sets the theme via the ThemeProvider. It doesn't seem to be a typescript (v5.2.2) upgrade that is causing this, but rather the upgrade to |
We do have our own type definition for the theme, but it does not comply with the shape of the boxShadow: (theme) => (theme as unknown as OurThemeDefinition).shadows.elevationElevated, which we would probably need to do in about 50+ places, and is something to maintain going forward, so I don't think it's a great solution. |
It puzzles me because I downgraded to 0.15, TS 4 and old @types/react, and I could still see the error in your repro, while I don't have it in my prod project. Anyway, try this ⬇️ Unfortunately, it only works at the top-level theme if you pass a function to |
Describe the bug
When upgrading to 0.16.1, we have introduced a bunch of TypeScript errors. An example of one such error, is when trying to access a string key on a
Scale
type property (e.gtheme.shadow
). An array index works as expected without any TS errors, but when trying to access a key (our shadow property in the theme is an object, not an array) we see a TS error.This can be solved with type assertions, but it would be quite cumbersome to update all instances like this.
E.g
To Reproduce
Steps to reproduce the behavior:
tsc --noEmit
(for example)theme.shadows
(as opposed to a numbered index)Expected behavior
The type
Scale
indicates that it can be an array or an object. Out of the box it seems to assume that any property of typeScale
is an array.Screenshots
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: