-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
fix: use tailwind.config.mjs
file by default config
#903
Conversation
🦋 Changeset detectedLatest commit: d51c9f9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👷 Deploy Preview for astro-starlight processing.
|
@astrojs/tailwind
use tailwind.config.mjs
file by defaultThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution 🙌
Some potential thoughts/questions I had while checking out the PR:
- I guess we would want to update the theme editor generated code too.
- I assume the Tailwind plugin JSDoc example would also need to be updated?
- Should the Starlight Tailwind Starter Kit (config file + README) also be updated?
Also, docs-only changes do not require a changeset, and even if we end up updating the Starlight Tailwind Starter Kit, I'm not sure if that would require a changeset either.
Thanks for picking this up @torn4dom4n! I think all of @HiDeoo’s suggestions here are good ones, and I can confirm we don’t need a changeset for these changes. If we update the JSDoc in the Tailwind plugin though, then we would want a patch changeset for the Tailwind package so that update gets released. |
tailwind.config.mjs
file by default config
Thank you @HiDeoo, I follow your suggestions. Happy to contribute to this awesome repo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks for the update. I guess we also want to switch from require
to import
too?
Note: there is also one require in the docs/src/components/theme-designer.astro
file that I cannot suggest change for it.
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
@HiDeoo I used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes 🙌
Yeah, I tried it too and was not expecting for it to work ^^ (but I guess it makes sense considering they're using jiti
under the hood).
Personally, I think this looks great and everything is covered. Let's see if others have any comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Just suggesting a small tweak to clarify the changeset.
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @torn4dom4n! LGTM 🚀
What kind of changes does this PR include?
Description
Since Astro v3.2.4, the
@astrojs/tailwind
integration now creates atailwind.config.mjs
file by default with PR withastro/astro#8638. So this content should be changed.