pacocoursey / next-themes

Perfect Next.js dark mode in 2 lines of code. Support System preference and any other theme with no flashing
https://next-themes-example.vercel.app/
MIT License
4.94k stars 177 forks source link

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

Open crammag opened 1 year ago

crammag commented 1 year ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂


Today I used patch-package to patch next-themes@0.2.1 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 commented 1 year ago

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

crammag commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Are there any updates for this?

st1ng7ay commented 12 months ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@paul-vd

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