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

refactor(module): provide tailwind config through template #1272

Merged
merged 10 commits into from
Feb 1, 2024

Conversation

ineshbose
Copy link
Member

@ineshbose ineshbose commented Jan 22, 2024

❓ 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

  1. This change will work with @nuxtjs/tailwindcss@^6.11.1.
  2. Configuration, especially with plugins, need to be passed through templates and not inline config. This will be beneficial for IntelliSense support as well.
  3. This change also prepares the module for first-class HMR support to @nuxtjs/tailwindcss (planned for 6.12.0); refer refactor: first class HMR supportΒ nuxt-modules/tailwindcss#795

Copy link

vercel bot commented Jan 22, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui βœ… Ready (Inspect) Visit Preview Feb 1, 2024 10:55am

@benjamincanac benjamincanac marked this pull request as draft January 22, 2024 22:40
@ineshbose ineshbose marked this pull request as ready for review January 25, 2024 13:47
@ineshbose
Copy link
Member Author

If we update @nuxt/ui-pro-edge and @nuxtjs/tailwindcss@6.11.1, it should work.

src/module.ts Outdated Show resolved Hide resolved
@ineshbose
Copy link
Member Author

Minor gotcha: nuxi prepare will be required on a fresh project; shouldn't do nuxi dev directly (but a restart can also fix); this may be an upstream @nuxtjs/tailwindcss improvement to wait for templates:generated hook to be completed to start resolving configPath

@ineshbose ineshbose changed the title refactor(tailwind-integration): provide configuration through template refactor(tailwind-integration): split file and provide config through template Jan 27, 2024
@benjamincanac benjamincanac merged commit f0ee189 into dev Feb 1, 2024
2 checks passed
@benjamincanac benjamincanac deleted the chore/tw-config-path branch February 1, 2024 14:06
@benjamincanac
Copy link
Member

@ineshbose I'm getting this when trying to run the docs locally:
CleanShot 2024-02-01 at 16 45 57@2x

@ineshbose
Copy link
Member Author

sure, give me a sec - testing it on my end.

@ineshbose
Copy link
Member Author

@benjamincanac seems OK on my end - but the traceback goes to docs/nuxt.config.ts where we have

const { resolve } = createResolver(import.meta.url)

I looked over at our package.json and docs/package.json which don't specify "type": "module" - should that fix?

Copy link
Member

The traceback goes to src/module.ts line 8 which is the import of installTailwind.

Copy link
Member

You're able to run the docs propertly on dev branch? πŸ€”

@ineshbose
Copy link
Member Author

You're able to run the docs propertly on dev branch? πŸ€”

I am - very odd!

I looked over at our package.json and docs/package.json which don't specify "type": "module" - should that fix?

This should definitely be the case though right, as Nuxt projects as ESM.

Copy link
Member

Does this mean that everyone using @nuxt/ui will have this error if they don't add "type": "module"?

Copy link
Member

Are you able to run dev:prepare too?

@ineshbose
Copy link
Member Author

Does this mean that everyone using @nuxt/ui will have this error if they don't add "type": "module"?

No, it is unlikely to hit this error; just referring to Nuxt starter templates, they would/should have it specified.

https://github.com/nuxt/starter/blob/606a503171717391f2aced7439cda4aee6cd6d55/package.json#L4

https://github.com/nuxt/starter/blob/module/package.json#L7

@ineshbose
Copy link
Member Author

Are you able to run dev:prepare too?

Yes - works on both my Ubuntu environment and Windows.

Some local change...?

Copy link
Member

No I don't have local changes, it's coming from this: https://github.com/nuxt/ui/blob/dev/src/tailwind.ts#L12.

If I checkout the commit right before, everything's fine.

Copy link
Member

Adding "type": "module" to both package.json didn't change a thing. Can you try with a clean install?

@ineshbose
Copy link
Member Author

trying on my macbook now..

@ineshbose
Copy link
Member Author

it's working πŸ€”

Can you send the output of npx nuxi info please?

Copy link
Member

I don't get it. Are you running the docs from the rootDir with pnpm run dev?

@ineshbose
Copy link
Member Author

Are you running the docs from the rootDir with pnpm run dev?

From root, yes, after doing pnpm i and pnpm dev:prepare.

@ineshbose
Copy link
Member Author

We can use something else in tailwind.ts for the resolve function too πŸ‘

@ineshbose
Copy link
Member Author

Was this PR not working before being merged?

Copy link
Member

Ok, I fixed it by restarting my mac, I'm so confused πŸ˜… Sorry about that!

@ineshbose
Copy link
Member Author

Copy link
Member

@ineshbose The error comes back after some time everytime I restart, will try to refactor this it's too painful.

@ineshbose
Copy link
Member Author

Okay let me know.

Copy link
Member

8b08ede

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