Shopify / hydrogen

Hydrogen lets you build faster headless storefronts in less time, on Shopify.
https://hydrogen.shop
MIT License
1.36k stars 262 forks source link

Setting up Tailwind ... documentation / maybe bugs #842

Open danabdn opened 1 year ago

danabdn commented 1 year ago

What is the location of your example repository?

No response

Which package or tool is having this issue?

Hydrogen

What version of that package or tool are you using?

2023.4.0

What version of Remix are you using?

1.15.0

Steps to Reproduce

Hello, I appreciate the time to look at this. I'm trying to set up a simple project. I started with the most basic template via the Hydrogen GitHub project creation (barebones sample, not the full demo store).

I'm finding a few things that are either outright confusing (which is a shame because you have built something amazing and I'm really excited!), or things that don't work like I would expect in a traditional Tailwind/React project.

Maybe it's by design. Maybe there are bugs. I'm sorry if I'm wrong - I'm new to Hydrogen. Maybe these problems will be similar for others, so hopefully looking at the below you make it easier for others on a similar journey.

Bug 1 (docs)

Preamble of confusion...

On https://shopify.dev/docs/custom-storefronts/hydrogen/building/begin-development The package json scripts section is...

"scripts": {
    "build": "npm run build:css && shopify hydrogen build --entry ./server",
    "build:css": "postcss app/styles/tailwind.css -o app/styles/tailwind-build.css --env production",
    "dev": "npm run build:css && concurrently -g -r npm:dev:css \"shopify hydrogen dev\"",
    "dev:css": "postcss app/styles/tailwind.css -o app/styles/tailwind-build.css -w",
    "preview": "npm run build && shopify hydrogen preview",
    "lint": "eslint --no-error-on-unmatched-pattern --ext .js,.ts,.jsx,.tsx .",
    "typecheck": "tsc --noEmit",
    "g": "shopify hydrogen generate"
  },

On https://github.com/Shopify/hydrogen/blob/dist/templates/demo-store-ts/package.json The package json script section is just...

 "scripts": {
    "dev": "shopify hydrogen dev",
    "build": "shopify hydrogen build",
    "preview": "npm run build && shopify hydrogen preview",
    "lint": "eslint --no-error-on-unmatched-pattern --ext .js,.ts,.jsx,.tsx .",
    "format": "prettier --write --ignore-unknown .",
    "format:check": "prettier --check --ignore-unknown .",
    "typecheck": "tsc --noEmit"
  },

Documentation confusion - why are there two very different ways of setting up a package.json? The second one is in a project that uses Tailwind.

Bug 2 (coding)

@apply only works if placed into Tailwind.css Does not work in css files that have been included via @import

Bug 3 (coding)

Tailwind typically works with scss out of the box - there seems to be "no loader" for SCSS. By design or bug?

Bug 4 (coding)

Unknown at rule @apply (just a lint error but not remedied by installing the Tailwind CSS Intellisense for VSCode. Same with @tailwind

Bug 5 (coding / documentation)

postcss.config.js in Minimal code template


module.exports = {
  plugins: {
    tailwindcss: {},
    autoprefixer: {},
  },
}

Yet on https://github.com/Shopify/hydrogen/blob/dist/templates/demo-store-ts It's the below but with no mention of autoprefixer. Is it a bug that autoprefixer has been omitted here? Or is autoprefixer automatic?

module.exports = {
  plugins: {
    'postcss-import': {},
    'tailwindcss/nesting': {},
    tailwindcss: {},
    'postcss-preset-env': {
      features: {'nesting-rules': false},
    },
  },
};

Expected Behavior

Bug 1 - if both these samples are inclusive of Tailwind the documentation should demonstrate one clean way of doing things? I don't see the advantage of one over the other. If there is a worthy / valid difference, can the documentation be updated with a note?

Bug 2 onwards

I would expect a single entry point Tailwind.css file with

@tailwind base;
@tailwind components;
@tailwind utilities;

// Then with 
@import "./base/base.css"
@import "./components/components.css"
@import "./components/utilities.css"

// Other custom styles
@import "./customStyles.css"

// Ideally SCSS working out of the box (nice to have)
@import "./generalWebsiteStyles.scss"

-- In those files imported, I would expect the @apply function of Tailwind to just work. Have compared configuration to a simple React/Tailwind application and cannot find any reason Hydrogen app shouldn't work the same way based on configs I can see in the Hydrogen project. Is there a configuration setting needed?

Similar problem with the postcss.config.js - seemingly two different ways to do something. Yet the one mentions autoprefixer, the other does not. Missing AutoPrefixer bug or now built in behind the scenes?

Actual Behavior

  1. Confusion as to how to configure a Hydrogen app - really not sure which configuration should be used and why.
  2. Unable to split css files and be able to use Tailwind's @apply . It's very handy to be able to split files out e.g. for responsive media queries, variables, themes, etc.
  3. SCSS does not work out of the box (nice to have)
  4. Output files from css - there should only be one not two (better performance to send one css file? - there is Tailwind.css and app.css). Also don't want to have to introduce a third one, or forth etc. for any custom files. Just one please (unless I'm wrong - just tell me if I'm wrong!)
  5. Missing postcss.config.js documentation?

First time submitting a "bug" - please be kind :-)

Thank you again! Dan.

frandiox commented 1 year ago

Hi Dan, thanks for bringing this up! It's indeed confusing so let me clarify a couple of things:

Hydrogen is now built on top of Remix, and it's actually the latter the one in charge of assets like CSS. The guide we have in the Hydrogen website shows the original way to use Tailwind in Remix. However, recently they released an unstable feature (unstable_tailwind: true in remix.config.js) to support Tailwind in a simpler way, and we updated our demo-store to use that. In the newest version (Remix 1.16), they have made this feature stable so we will be gradually updating guides. (cc @scottdixon ).

If you want to use Tailwind right now, I'd recommend doing what you see in the demo-store template (the link above shows how). It will be easier to migrate to the latest version of Remix once Hydrogen supports it.

apply only works if placed into Tailwind.css

I think this should be supported when using the new feature (unstable_tailwind: true in remix.config.js). Can you double check in your project and let us know?

Tailwind typically works with scss out of the box - there seems to be "no loader" for SCSS. By design or bug?

This is probably by design in Remix. They mention to process your files beforehand. However, since now they support PostCSS, perhaps you could give postcss-sass a spin and see if that works.

Unknown at rule https://github.com/apply (just a lint error but not remedied by installing the Tailwind CSS Intellisense for VSCode.

You can try some of the workarounds listed in this Tailwind repo discussion.

It's the below but with no mention of autoprefixer. Is it a bug that autoprefixer has been omitted here? Or is autoprefixer automatic?

I'm not 100% sure since it seems this behavior has been modified several times. I think it should be automatic but we'll double check this again soon. It can't hurt to add it manually, in any case. -- Edit: I just remembered, the reason we don't add it in the demo-store it's because it's already included in postcss-preset-env. If you don't use that, please add autoprefixer manually.


We are going to add more scaffolding features to the Hydrogen CLI soon, and one of them will be adding Tailwind to a new project. We will also update our guides once this is ready.

danabdn commented 1 year ago

Hi @frandiox

Thank you - I appreciate you taking the time to provide clarity and for considering my report which might not fall into the traditional bug/fix scenario.

Yes, we are very reliant on the documentation as there are a lot of considerations (as well as unfamiliar / cutting edge tech) to get one's head around, but having the demo-store as the most reliable source of reference is very helpful to know.

From your reply

I have updated my configuration based on the github change summary - so useful - ta.

Remix 1.16 looks like quite a few major changes - I think I had better wait until the demo store is updated otherwise I'm going to break things - it's a natural talent :-)

For SCSS - I'll take a look at postcss-sass, cheers.

Having CLI updates for new projects will be good, I don't use the CLI as I prefer to understand any changes being made. The Shopify admin offers a process of getting started that which uses the barebones / demo-store. That's the process I'm personally following i.e. copy, explore, understand, modify. But I'll take a look at the CLI and make sure I'm not missing productivity tricks / features.

Thanks for the autoprefixer info. Nice to know it's there somewhere!

Feedback

Tailwind future flags - this allowed @apply to work. Yay!

However, unless I've missed something, I cannot get layers to work e.g. @layer base {}, @layer components {}, @layer utilities.

Plus, I can only reference other files, not from tailwind.css, but it has to be from within app.css. Without the @layer attribute working, then I believe all code comes in at the end. I'm not that familiar with Tailwind's inner workings yet, but those directives might be useful to use for the ordering of e.g. components styles vs utility styles.

I presume the @layer directive will work in Remix 1.16?

Happily ignoring lint errors which are no longer breaking, although still underlined.

Questions

  1. Will you announce somewhere that you've updated the demo-store to remix 1.16? E.g. developer blog or should I just keep an eye out on Github?
  2. If I wanted to update to Remix 1.16 can I simply follow the guide you linked to in your reply or is it more involved and maybe you would recommend I should wait for the docs to be updated?

Now that apply is working and I can split up my files and that's a big obstacle removed (thank you!). Then, yes, I can go back and update my code with Remix 1.16 so it's more Tailwind friendly.

Anything I've said that's wrong - do feel free to correct me or provide extra info / pointers. Much appreciated.

Many thanks for your help. Dan.

Update: I may have spoke too soon - I'm slowly importing the demo-store files (and removing the old code that was based on the Shopify Hydrogen tutorial documentation) so I can learn. But I did receive the following error which means my fears were realised... (note: I was trying to create a component with tailwind styles - for some reason the output wasn't exporting tailwind styles e.g. bg-blue-500 didn't make it to the tailwind-build.css). All root configs same as demo-store-ts.

app/root.tsx:22:19: ERROR: [plugin: css-file] Build failed with 1 error:
node_modules/postcss/lib/input.js:148:15: ERROR: [plugin: postcss-plugin] C:\GitHub\COAD\hydrogen\hydrogen\app\styles\base\base.css:10:1: `@layer base` is used but no matching `@tailwind base` directive is present.

If you have any suggestions - I'm all ears - will post an update when I've finished importing and testing. It may be a case of waiting for 1.16 though.

danabdn commented 1 year ago

Just a quick update: I am updating my local project to the latest demo-store-ts template. Will feedback on the Tailwind issues soon. However in the meantime I wanted to report some unexpected behavior when updating to the latest template (that was updated just a few days ago).

I spent the evening copying and pasting over the code (I know, I know but it's just a way for me to familiarise myself with the code). Added just the index page. It worked for a moment and then routing errors started (errors every half a second from one page). This was a valid routing error because I had not included the countries API route. So I added it.

Added the rest of the routes and but then continued to get the error complaining about the the route and countries api and referencing the route $ as an issue as though it was interpretting countries api as a catchall from the $ (or $_) route. Sorry I cleared my terminal otherwise I would have shared.

Explored and compared code for a few hours to find out what might be causing it (presuming human error). Couldn't see anything that didn't match the live demo store. I had removed all old code files before I started the copying process.

So, since the error, I changed no code. Confirmed all configuration settings were correct including Package.json, ran package install twice. Restarted Ctrl + C the dev server multiple times.

No matter what I did I just kept getting a route error.

Next day and PC reboot - it suddenly started to work as expected.

It's like restarting the local dev server via Ctrl + C then npm run dev didn't seem to properly restart the service.

I believe there may be a problem e.g.

  1. Caching between npm run dev's
  2. A Windows service for the e.g. CLI/server does not restart fully and therefore caches previous result e.g. if there was an error.
  3. Some other reason that means the local dev server has not fully restarted / running off latest code.

Is there a command to fully restart the local dev server or prevent any caching from previous runs of npm run dev?

Deleting countries api route, running the server, putting it back, restarting server is the only way I might try to reproduce this error (but not tried it yet) - just thankful it's working as limited knowledge of remix.

There may be no big issue to solve, but if you are at least aware of it, it may be that others have this issue too and then it becomes worth looking into.

Will keep you posted on the original Tailwind issue in a few days.

Thanks, Dan.

danabdn commented 1 year ago

Just a quick update - sorry been busy running business - still delving into this.

Update 1 - I found that some tailwind styles were not re-ordering - after adding prettier-plugin-tailwindcss (dev tool) it worked fine.

Update 2 - CSS caused errors when importing from demo-store - it looks like the demo-store app.css is using scss / sass in app.css?? Or am I missing a config that would make this error free css?

.prose { h1, h2, h3, h4, h5, h6 { &:first-child { @apply mt-0; } } }

Update 3 - Opened up a stack overflow question regarding @importing tailwind custom styles

See Stack Overflow question

Will keep you posted.

Thanks, Dan.