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

fix: Handle ESM Tailwindconfigs (fix #667) #723

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

schwarmco
Copy link
Contributor

Description

Fixes #667

When using ESMs export default instead of module.exports in tailwind.config.{js,ts} the tailwindConfig is nested in a default key which then incorrectly merges with tailwinds default config like this:

image

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • If it's a new feature, provide a convincing reason to add it. Ideally, you should open a suggestion issue first and have it approved before working on it.
  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link

stackblitz bot commented May 26, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

codesandbox bot commented May 26, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

netlify bot commented May 26, 2024

Deploy Preview for histoire-examples-svelte3 ready!

Name Link
🔨 Latest commit 853dc2e
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-svelte3/deploys/666be7db9aa7530009bc2dea
😎 Deploy Preview https://deploy-preview-723--histoire-examples-svelte3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 26, 2024

Deploy Preview for histoire-examples-vue3 ready!

Name Link
🔨 Latest commit 853dc2e
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-vue3/deploys/666be7db4bcd9d0008dfb793
😎 Deploy Preview https://deploy-preview-723--histoire-examples-vue3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 26, 2024

Deploy Preview for histoire-controls ready!

Name Link
🔨 Latest commit 853dc2e
🔍 Latest deploy log https://app.netlify.com/sites/histoire-controls/deploys/666be7db7d19ce0008b3e67b
😎 Deploy Preview https://deploy-preview-723--histoire-controls.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 26, 2024

Deploy Preview for histoire-site ready!

Name Link
🔨 Latest commit 853dc2e
🔍 Latest deploy log https://app.netlify.com/sites/histoire-site/deploys/666be7db4221bb00082d38fc
😎 Deploy Preview https://deploy-preview-723--histoire-site.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@bahulneel bahulneel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch didn't work for me as the tailwind plugins (i.e. backgroundColours, etc.) that the tailwind storyTemplate expects were not being generated. After reading the documentation, I noticed that resolveConfig needed to run on with the tailwind config at the base of the imported object, not just the storyTemplate function.

@schwarmco
Copy link
Contributor Author

Thank you for the feedback @bahulneel - i didn't run into that problem but i guess it depends on the ts environment you're in. As your suggestions were optional chained i've pulled them in the PR.

Copy link

@waldemarennsaed waldemarennsaed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO that looks correct ... well done.

@waldemarennsaed
Copy link

Anything blocking from merging this?

@waldemarennsaed
Copy link

waldemarennsaed commented Jul 17, 2024

This would be nice to be merged. I have the current issue of:

  • having a tailwind config that overwrites tailwind colors and other theme-related stuff
  • only working if exporting tailwind as module.exports

Then, I need to import my tailwind theme object in JavaScript, e.g. in one of my stories like that:

<script setup lang="ts">
import resolveConfig from 'tailwindcss/resolveConfig'
import tailwindConfig from '../../../../tailwind.config.js'

const fullConfig = resolveConfig(tailwindConfig)
</script>

Resulting in the following error:
Bildschirmfoto 2024-07-17 um 09 20 10
Bildschirmfoto 2024-07-17 um 09 22 49

I am pretty sure that this MR will fix this issue for me, since I will be able to keep a correct Tailwind Story and will be able to export my tailwind config as ESM.

@waldemarennsaed
Copy link

@schwarmco not intending to annoy anyone but ... any blockers on this? Or this be merged?

@schwarmco
Copy link
Contributor Author

not annoying at all @waldemarennsaed - i'd love to get this merged too but i'm not a maintainer of histoire. i am actively using that patch on my projects

@waldemarennsaed
Copy link

@Akryum I don't know who else I could mention here - can you maybe bring some life to this MR? 🤓

@bjalili-aedifion
Copy link

This would be nice to be merged. I have the current issue of:

  • having a tailwind config that overwrites tailwind colors and other theme-related stuff
  • only working if exporting tailwind as module.exports

I am also facing this problem🥹👆🏻

And I think this MR will help:

I am pretty sure that this MR will fix this issue for me, since I will be able to keep a correct Tailwind Story and will be able to export my tailwind config as ESM.

@Akryum Akryum merged commit 7b6389f into histoire-dev:main Aug 22, 2024
12 of 13 checks passed
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.

Support tailwind.config.ts
5 participants