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

generate css again #281

Open
wants to merge 7 commits into
base: hakyll
Choose a base branch
from

Conversation

hazelweakly
Copy link

While preparing to work on some small changes to the CSS I went over with David earlier (one line of CSS, actually), I realized that there was a lot of complexity around the current CSS.
Notably, we were using tailwind but not actually generating the files anymore, none of the nix code was used, and the minified CSS file had some manual changs to it.

So, I reverse engineered all(-ish) of the changes, re-setup all of the nodejs stuff exactly as it used to be (including outdated versions of most everything), and went from there.
This necessitated some questionable changes to the tailwind config and I attempted to document everything thoroughly.

Additionally, I do some somewhat Clever(tm) things so that nobody ever has to actually have nodejs installed locally unless they need to change the tailwind.css file and test that out.

Now, hopefully, further "just let me write CSS normally already" changes should be relatively easy and straightforward to do (it's in the main.css file and you don't have to worry about it).

The main.css file currently doesn't get minimized or optimized but, then again, neither does most anything else. That can be tackled later (there's lower hanging fruit to address first).

And finally, I'll submit a follow up PR for... one grand and glorious line of CSS. The one I originally intended to write this morning :)

Copy link
Member

@david-christiansen david-christiansen left a comment

Choose a reason for hiding this comment

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

I like that people can work on the site without having Node installed - that makes this accessible to a wider variety of skill sets. And it's good to have the Tailwind config manifested again, so we're not stuck hand-hacking around its lack. Thanks!

It's still very unclear to me how often this will be an issue, though. My understanding is that the Tailwind compiler is a kind of "CSS treeshaker" that outputs the subset of the overall Tailwind setup that's needed for the site in question, perhaps expanding some configuration macros along the way. Is that right? Does that mean that those who want to work on the style of the site will need Node, but those who want to work on the content will be OK without it? I know I was recently missing one of the Tailwind abbrevs from its documentation, which makes me think that I'd need to re-run the compiler.

What is the workflow here? There's no explicit documentation of how to work on the site that I can see, and powerful kludges like this should come with instructions! E.g. a section in the README that says something ilke:

This site uses Tailwind, which is a CSS framework that includes a custom preprocessor that eliminates the parts of the framework not used by the site at build time. Tailwind mostly consists of abbreviations for various inline styles, and it is intended to be used by scattering them around each individual HTML element. If your changes neither modify CSS nor add previously-unused Tailwind abbreviations, then you probably don't need to re-run the Tailwind compiler. In this case, your workflow is XYZ. However, if you are modifying the appearance or adding new Tailwind abbreviations, it will be necessary to regenerate the site's CSS file. To do so, follow the following procedure: ZYZ.

I know that I'd be overwhelmed trying to work all this out when I have a short deadline on the site.

Thank you for helping get this unstuck!

-- We concatenate a dev.css file that exists at the root of the repository so that people don't
-- need to have nodejs setup or working in order to get a functional development experience
-- "why yes this is very crimes why do you ask"
compile $ getResourceString >>= traverse (unixFilter "cat" ["-", "dev.css"])
Copy link
Member

Choose a reason for hiding this comment

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

Won't this make it not work on Windows? I'd like Windows users to be able to contribute here.

Copy link
Author

Choose a reason for hiding this comment

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

It may work on Windows? I'm not sure.

That said, this was the ugliest/quickest possible thing I could think of. I will put a few calories into actually writing a real solution here to make sure it works on windows :)

compile $ getResourceString >>= traverse (unixFilter "cat" ["-", "dev.css"])

-- Here is where interop with JS would happen if we felt like making every
-- haskell developer working on this site learn all of the nodejs nonsense
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- haskell developer working on this site learn all of the nodejs nonsense
-- Haskell developer working on this site learn node.js

I don't want our source code to contain negative statements about any programming language, especially ones that aren't Haskell

Copy link
Author

Choose a reason for hiding this comment

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

Very valid! I will change this :)

@@ -239,7 +249,7 @@ main = hakyll $ do
--
-- This identifier compiles the body the file to plain text, to be used in the OpenGraph description field

match "**/*.markdown" $ version "description" $ compile pandocPlainCompiler
match ("**/*.markdown" .&&. (complement "node_modules/**/*.markdown")) $ version "description" $ compile pandocPlainCompiler
Copy link
Member

Choose a reason for hiding this comment

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

Can node_modules live somewhere else, perhaps? For instance, could we put the Hakyll stuff in one subdirectory, and the Node stuff in another? This seems like a big footgun for future work on the site.

Copy link
Author

Choose a reason for hiding this comment

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

node_modules is generally not something that can be moved, unfortunately. I might be able to move it and do something different with the package setup (at the cost of making things more unusual on the nodejs side of things). I'll think more about this: initially I wanted to write this in a way that "match" automatically ignored everything in gitignore but that seemed a bit difficult to accomplish with minimal changes.

I'll think of a way to solve this in a manner that avoids footguns.

"w-800": '800'
},
borderWidth: {
"3": "3px",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"3": "3px",
"3": "3px"

For consistency.

* below is a hardcoded output of tailwind + normalize so that the site preview
* works without requiring interop with nodejs or the tailwindcss compiler.
*
* IMPORTANT: this is *not* the CSS that you will see in production!
Copy link
Member

Choose a reason for hiding this comment

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

This strikes me as an accident waiting to happen.

I don't think this comment will be very visible when they get out of sync. Perhaps CI should check that this is still the output of tailwind + normalize?

Copy link
Author

Choose a reason for hiding this comment

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

We can do that! I'd remove the comment then and just make it the entire minified file (which I can do pretty easily). I'll add that to the list of things I address.

@@ -0,0 +1,21 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we can document what this file is, or alternatively place all the Node and Tailwind stuff in its own directory that can have a README? I worry that mixing build systems like this is going to make things confusing.

I think the long-term solution is to ditch Tailwind, but that seems quite time consuming at the moment unfortunately.

Copy link
Member

Choose a reason for hiding this comment

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

Too bad JSON doesn't support comments!

Copy link
Author

Choose a reason for hiding this comment

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

I would agree. I think I should be able to put quite a lot of the tailwind and nodejs stuff in its own directory, especially since I "only" need the tailwind CLI + postcss and can invoke them directly. In fact, I think that will solve quite a few issues: I'll investigate this approach to make sure it works. Thanks for the idea!

}
}
@media (min-width: 640px) {
.sm\:border-gray-300 {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with the backslash syntax for CSS selectors here, and a bit of Googling for what they mean doesn't turn anything up. Could this be a bug in the script that produced it?

Copy link
Author

Choose a reason for hiding this comment

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

It's indeed intentional. Tailwind has the ability to write classes like a:b in css and it is technically allowed, however the backslash is required to escape the : so that it isn't treated as a field separator token.

(To clarify: This CSS has always existed. I just un-minified it so that people can actually read it. If you'd like, I can re-minify it so that it's not a "long" file, but that wouldn't change anything functionally and
it makes updates of dev.css inscrutable in CI/diffs)

* NOTE: The underlying implementation details of how this works are as follows
* 1. tailwindcss needs these directives in order to know where to generate the
* output
* 2. first, hackyll is run and will copy this file to
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* 2. first, hackyll is run and will copy this file to
* 2. first, hakyll is run and will copy this file to

* output
* 2. first, hackyll is run and will copy this file to
* _sites/assets/tailwind.css
* 3. then, after hackyll is done, postcss will be ran on _this_
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* 3. then, after hackyll is done, postcss will be ran on _this_
* 3. then, after hakyll is done, postcss will be ran on _this_

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* 3. then, after hackyll is done, postcss will be ran on _this_
* 3. then, after hackyll is done, postcss will be run on _this_

* tailwindcss. tailwindcss (the compiler) will scan all the files in the
* site for tailwind classes (including _this_ assets/tailwind.css file).
* 4. postcss's output file is configured to be _site/assets/tailwind.css
* (which causes this file to get overwritten with the correct contents)
Copy link
Member

Choose a reason for hiding this comment

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

Not this file, right? It sounds like it should overwrite the one in _site, not this one. Or did I misunderstand?

Copy link
Author

Choose a reason for hiding this comment

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

You are correct. I'll fix that :)

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.

None yet

2 participants