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

More type-safe theme, themes and resolvedTheme? #149

Open
crammag opened this issue Nov 7, 2022 · 9 comments
Open

More type-safe theme, themes and resolvedTheme? #149

crammag opened this issue Nov 7, 2022 · 9 comments
Labels
enhancement New feature or request v1 Planned for v1

Comments

@crammag
Copy link

crammag commented Nov 7, 2022

Hi! 👋

Firstly, thanks for your work on this project! 🙂


Today I used patch-package to patch [email protected] for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/next-themes/dist/types.d.ts b/node_modules/next-themes/dist/types.d.ts
index 1818dd9..aec32a5 100644
--- a/node_modules/next-themes/dist/types.d.ts
+++ b/node_modules/next-themes/dist/types.d.ts
@@ -4,15 +4,15 @@ interface ValueObject {
 }
 export interface UseThemeProps {
     /** List of all available theme names */
-    themes: string[];
+    themes: ('dark' | 'light' | 'system')[];
     /** Forced theme name for the current page */
     forcedTheme?: string;
     /** Update the theme */
     setTheme: (theme: string) => void;
     /** Active theme name */
-    theme?: string;
+    theme?: 'dark' | 'light' | 'system';
     /** If `enableSystem` is true and the active theme is "system", this returns whether the system preference resolved to "dark" or "light". Otherwise, identical to `theme` */
-    resolvedTheme?: string;
+    resolvedTheme?: 'dark' | 'light' | 'system';
     /** If enableSystem is true, returns the System theme preference ("dark" or "light"), regardless what the active theme is */
     systemTheme?: 'dark' | 'light';
 }

This issue body was partially generated by patch-package.


As you can see I've added a more robust typing for themes, theme and resolvedTheme, I wonder if this is something that could be updated at package level instead of just patching it for my needs. Is there any other value for those properties and that's the reason because it's a string?

In case this could be added to the library, a better way (or at least I think so) would be using a type like:

export type ThemeOptions = 'dark' | 'light' | 'system';

And using it for the properties on the interface.

Thank you!

@pacocoursey
Copy link
Owner

pacocoursey commented Nov 7, 2022

Yeah, any theme can be used (https://github.com/pacocoursey/next-themes#more-than-light-and-dark-mode) so your patch would be a little too limiting. But I'm sure there's a solution with generics here, will leave this issue open to improve it

@pacocoursey pacocoursey added the enhancement New feature or request label Nov 7, 2022
@pacocoursey pacocoursey changed the title It is possible to modify the typings to be more type-safe on theme, themes and resolvedTheme? More type-safe theme, themes and resolvedTheme? Nov 7, 2022
@crammag
Copy link
Author

crammag commented Nov 7, 2022

Oh, that's true, I should have watched more carefully the docs 😅... But I'm glad you found a way to take this option into account too, ty!

@crammag
Copy link
Author

crammag commented Nov 10, 2022

I've opened a PR with what I've understood from your comment, maybe we can iterate from there when you have time to review it :)

@AhmedBaset
Copy link

Are there any updates for this?

@st1ng7ay
Copy link

Yeah, any theme can be used (https://github.com/pacocoursey/next-themes#more-than-light-and-dark-mode) so your patch would be a little too limiting. But I'm sure there's a solution with generics here, will leave this issue open to improve it

no solution ?

@paul-vd
Copy link

paul-vd commented Jul 18, 2024

Yeah, any theme can be used (pacocoursey/next-themes#more-than-light-and-dark-mode) so your patch would be a little too limiting. But I'm sure there's a solution with generics here, will leave this issue open to improve it

Why not allow to extend the declaration types, similar to next-auth

So it would look something like this in the types/next-theme.d.ts

declare module "next-theme" {
  export type Themes = "dark" | "light" | "acme-theme"
}

Otherwise yes, the simple solution is to have generics, but that means you need to pass it each time when using the hook, whereas with the types declaration it will be global, so better DX.

@paul-vd
Copy link

paul-vd commented Jul 18, 2024

Here is an example how I resolved it for our project where I know we only have a dark or light theme:

declare module "next-themes" {
  import { UseThemeProps } from "next-themes/dist/types.ts";
  export { ThemeProvider } from "next-themes/dist";

  type AugmentedUseThemeProps = UseThemeProps & {
    themes: ("dark" | "light" | "system")[];
    resolvedTheme?: "dark" | "light";
    theme?: "dark" | "light" | "system";
    systemTheme?: "dark" | "light";
  };

  export const useTheme: () => AugmentedUseThemeProps;
}

@musjj
Copy link

musjj commented Aug 11, 2024

@paul-vd

I'm getting a Cannot redeclare block-scoped variable 'useTheme'. ts(2451) error. Is there a way to get around this?

@paul-vd
Copy link

paul-vd commented Sep 19, 2024

@paul-vd

I'm getting a Cannot redeclare block-scoped variable 'useTheme'. ts(2451) error. Is there a way to get around this?

@musjj What ts version are you on and what does you tsconfig.json look like?

@trm217 trm217 self-assigned this Nov 7, 2024
@trm217 trm217 added the v1 Planned for v1 label Nov 7, 2024
@trm217 trm217 removed their assignment Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v1 Planned for v1
Projects
None yet
Development

No branches or pull requests

7 participants