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

feat(module): handle tailwindMerge config from app.config #2902

Merged
merged 13 commits into from
Dec 26, 2024

Conversation

stijns96
Copy link

@stijns96 stijns96 commented Dec 14, 2024

πŸ”— Linked issue

Resolves #1939

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR introduces the ability to override/extend tailwind-merge behaviour through the app.config. This is necessary when overriding the tailwind.config default values as explained here: https://github.com/dcastil/tailwind-merge/blob/v2.1.0/docs/configuration.md#usage-with-custom-tailwind-config

More details on how to override the config: https://github.com/dcastil/tailwind-merge/blob/v2.1.0/docs/configuration.md#extending-the-tailwind-merge-config

When extending the spacing for example in your tailwind.config.ts:

import type { Config } from 'tailwindcss'

export default <Partial<Config>>{
  theme: {
    extend: {
      spacing: {
        sm: '0.5rem',
        base: '1rem',
        lg: '2rem'
      }
    }
  }
}

You can now configure tailwind-merge to merge those values with each other.

import { theme } from "#tailwind-config";

export default defineAppConfig({
  ui: {
    tailwindMerge: {
      override: {
        theme: {
          spacing: [...Object.keys(theme.spacing)],
        },
      },
    }
  },
})

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stijns96
Copy link
Author

Hi @benjamincanac,

I decided to ditch the restored branch and created fix/extend-tailwind-merge as the old one already contained 25 merge conflicts.

Additionally, I think it's almost fully working, but there is still one challenge. It looks like tailwind-merge doesn't export a type for the extendTailwindMerge function which I want to use for the app.config.ts. I opened a discussion on their repo.

I would like to to wait to merge this when we at least have an answer on this

@stijns96
Copy link
Author

stijns96 commented Dec 16, 2024

Hi @benjamincanac

I found a "workaround". The only annoying thing is that it suggest to types... The normal one at least is working, but I can't figure out how to get rid of the second one. TW Merge haven't replied yet.

image image

./playgroud/src/module.ts

type TailwindMergeConfig = Parameters<typeof extendTailwindMerge>[0];

type UI = {
  primary?: string
  gray?: string
  colors?: string[]
  strategy?: Strategy
  tailwindMerge?: TailwindMergeConfig
  [key: string]: any
} & DeepPartial<typeof config, string | number | boolean>

Any thoughts on this?

@benjamincanac
Copy link
Member

@stijns96 Seems fine! I just did the same work for v3: #2938

@stijns96
Copy link
Author

stijns96 commented Dec 19, 2024

@benjamincanac cool! Should I undraft the PR? When do you expect it to be released? I really need it in my MVP...

Let me know!

P.s. have to add some docs as well

src/module.ts Outdated Show resolved Hide resolved
@benjamincanac benjamincanac changed the title fix(module): handle tailwindMerge config from app.config feat(module): handle tailwindMerge config from app.config Dec 19, 2024
@benjamincanac benjamincanac marked this pull request as ready for review December 19, 2024 13:49
@benjamincanac
Copy link
Member

I don't know when will the release happen but once it's merged on dev, you will be able to use the edge version anyway.

@benjamincanac
Copy link
Member

Do you plan to make the docs in this PR?

@stijns96
Copy link
Author

@benjamincanac I think that might be the easiest to do. I'll try to do that asap (hopefully tonight).

Copy link
Member

benjamincanac commented Dec 19, 2024

I guess you can put it somewhere in https://ui.nuxt.com/getting-started/theming#components. I'll have to do it also on my PR for v3.

@stijns96
Copy link
Author

I guess you can put it somewhere in https://ui.nuxt.com/getting-started/theming#components. I'll have to do it also on my PR for v3.

Did you really mean under the "Components" or as a dedicated toc item? E.g.

  • Components
  • Extend Tailwind Merge
  • Dark mode

Copy link
Member

I would say under Components yes as we mention tailwind-merge inside ui prop and class attribute.

…figurations

This update introduces documentation on how to extend Tailwind Merge to accommodate custom Tailwind CSS configurations, including examples for `app.config.ts` and `tailwind.config.ts`. This enhancement aims to improve user understanding and implementation of Tailwind Merge in their projects.
This commit refactors the module by removing the redundant import of `extendTailwindMerge` and consolidating the type definition for `TailwindMergeConfig`. This change enhances code clarity and maintains consistency in type usage.
This commit simplifies the module by eliminating the duplicate import of `extendTailwindMerge`, enhancing code clarity and consistency.
@stijns96
Copy link
Author

@benjamincanac There you go

image image

@stijns96
Copy link
Author

Copy link
Member

@stijns96 Do you want to wait for their release?

@stijns96
Copy link
Author

@benjamincanac was thinking the same. Let me check with them when the are planning a release

@benjamincanac
Copy link
Member

@stijns96 Should be good!

…efinitions in module

This commit updates the `tailwind-merge` dependency to version 2.6.0 in both `package.json` and `pnpm-lock.yaml`. Additionally, it refines the type definitions in `src/module.ts` by replacing the import of `extendTailwindMerge` with a more specific type definition for `tailwindMerge`, enhancing type safety and clarity.
@stijns96
Copy link
Author

@benjamincanac,

Has been released and PR has been updated. Works like a πŸ€ now!

@benjamincanac benjamincanac merged commit ea15e21 into nuxt:dev Dec 26, 2024
2 checks passed
@benjamincanac
Copy link
Member

Thanks @stijns96! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants