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

MantineThemeColors are possibly "undefined" (loses types on TypeScript v4.8.2) #2364

Closed
maxijonson opened this issue Sep 6, 2022 · 10 comments
Labels
help wanted Contributions from community are welcome

Comments

@maxijonson
Copy link

maxijonson commented Sep 6, 2022

What package has an issue

@mantine/core

Describe the bug

I have a project that uses TypeScript 4.7.4 and compiles just fine. However, I recently noticed VS Code (which is using TypeScript 4.8.2) has started reporting errors for MantineThemeColors. At first, I thought it was a problem with my VS Code, but after upgrading my project's TS version to 4.8.2, the compilation started failing.

Running tsc --version && tsc --no-emit with TS 4.7.4 (no errors):

Version 4.7.4

Running tsc --version && tsc --no-emit with TS 4.8.2:

Version 4.8.2
theme.colors.dark[9]
                ^
Object is possibly "undefined".

tsconfig.json

{
    "compilerOptions": {
        "incremental": true,
        "target": "ESNext",
        "module": "ESNext",
        "outDir": "./dist",
        "moduleResolution": "node",
        "esModuleInterop": true,
        "allowSyntheticDefaultImports": true,
        "skipLibCheck": true,
        "resolveJsonModule": true,
        "baseUrl": ".",
        "allowJs": true,
        "sourceMap": true,
        "preserveWatchOutput": true,
        "pretty": true,
        "newLine": "LF",
        "types": ["node"],
        "jsx": "react",
        "strict": true,
        "exactOptionalPropertyTypes": false,
        "noFallthroughCasesInSwitch": true,
        "noImplicitAny": true,
        "noImplicitOverride": true,
        "noImplicitReturns": true,
        "noUncheckedIndexedAccess": true,
        "noUnusedLocals": false,
        "noUnusedParameters": false,
        "forceConsistentCasingInFileNames": true
    },
    "include": ["src", "vite.config.ts"]
}

What version of @mantine/hooks page do you have in package.json?

5.2.5

If possible, please include a link to a codesandbox with the reproduced problem

Unable to create a codesandbox, since they don't seem to allow running NPM scripts and the IDE's TS version is stuck at 4.5.5 .

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

Removing (string & {}) from DefaultMantineColor seems to fix the type (i.e. I don't get "possibly undefined" errors). However, it causes errors in other places, such as ThemeIcon.color which would no longer accept a string. (I'd sometimes use gray.0 instead of the theme object from useMantineTheme. There could be other errors that I have not encountered.

The alternative right now would be to simply use optional chaining.

theme.colors.dark?.[9]
@rtivital
Copy link
Member

rtivital commented Sep 6, 2022

Removing (string & {}) is not an option, it will break a lot of other things. I do not know how to fix that for strict mode atm, PR welcome.

@rtivital rtivital added help wanted Contributions from community are welcome and removed review required labels Sep 6, 2022
@maxijonson
Copy link
Author

Alright, I'll try to check it out. But clearly, if it used to work before and now it doesn't, something changed on TypeScript's side for it to break. It could possibly be a bug on TypeScript's side too, I just wouldn't be able to pinpoint how.

@PointSingularity
Copy link
Contributor

There was a change in 4.8 that concerns the "string & {}"

But according to the PR "string & {}" should be an exception, and the functionality of it should have been preserved.

@maxijonson
Copy link
Author

I tried a super minimal TS project with the concerned types to better isolate the issue and at first, the error never occurred (on both versions). I then plugged my tsconfig and saw the issue come back under 4.8.2. After going one by one on each rule, I found that the culprit was here:

      "noUncheckedIndexedAccess": true,

This rule automatically adds undefined when accessing an un-declared index. But that is just the issue, dark IS a declared index, so accessing it without optional chaining should work just fine.

@rtivital
Copy link
Member

rtivital commented Sep 8, 2022

Alright, then I think it is fine from our side with default strict mode settings and "noUncheckedIndexedAccess": true, should be turned off.

@PointSingularity
Copy link
Contributor

I tried a super minimal TS project with the concerned types to better isolate the issue and at first, the error never occurred (on both versions). I then plugged my tsconfig and saw the issue come back under 4.8.2. After going one by one on each rule, I found that the culprit was here:

      "noUncheckedIndexedAccess": true,

This rule automatically adds undefined when accessing an un-declared index. But that is just the issue, dark IS a declared index, so accessing it without optional chaining should work just fine.

Great find @maxijonson! You should definitely submit a new issue with your minimal reproduction to the TypeScript GitHub, because it does seem like it should work.

@maxijonson
Copy link
Author

Good point, I'll take a moment today to look through some issues and make sure this isn't already reported. If not, I'll submit an issue. In any case, I'll give feedback here on where the issue is so we can track it!

@maxijonson
Copy link
Author

I've created the related issue here

@rtivital
Copy link
Member

rtivital commented Sep 8, 2022

I'm closing this issue as it seems not to be related to Mantine, if you think that there is something to be done on our side, then feel free to reopen it.

@rtivital rtivital closed this as completed Sep 8, 2022
@maxijonson
Copy link
Author

A PR has been created in TypeScript and merged, this will be fixed in 4.9.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions from community are welcome
Projects
None yet
Development

No branches or pull requests

3 participants