Deprecate/remove generate-development-css script and update CONTRIBUTING.md

[reedspool]

The Contributing Doc suggests one should use ./build-tools/heroku/tailwind/generate-development-css during development, and then (in the same dir) generate-css before merging.

But in almost a year, no one has thought to add the really-very-useful --watch mode to generate-development-css, after it was added to the generate-css script. That signals to me that no one is actually using the script, making it useless cruft and misleading to newbies like me as that's what the docs say to use.

Please let me know if I'm wrong and this script is in use or does something useful that the other script doesn't do. If my hunch is correct, I'll gladly make a PR to remove the unused script and update the contributing doc.

[Felienne]

Good catch @reedspool, I think there might be a use for this but I am not sure. @rix0rrr will know I think!

[rix0rrr]

Hey @reedspool, thanks for the interest in improving the developer experience!

That signals to me that no one is actually using the script

Well, I use it all the time 😅 but I agree that it is confusing to have two scripts, especially since their functions are very similar and a computer could figure out when it needs to do one thing and when the other.

Here is the the reason for this script existing (if this rationale is not in the contributing guide, we should probably add it):

Our goal currently(*) is that a developer should only need git and Python, and only need to check out the repository and run python app.py to get up and running with Hedy. This is to encourage contributions with a minimum of hurdles. That means that we can't require any additional build steps to be run before starting local development. That in turn means that generated artifacts should be committed to the repository in a "good to go" state.

Tailwind has "strip unused CSS classes" feature to minimize the size of the CSS file, which is great. But if we commit a stripped version of the CSS to git, then any time you want to use a new class that hasn't been used before, your HTML will not work and you need to regenerate the CSS file. So we have decided to commit the multi-megabyte "full" CSS File to git, so that, while developing, all classes are available. The minimized file will be generated on deployment to the server.

It's generate-development-css's job to generate the full file, and generate-css's job to generate the stripped file. Why someone has added --watch to a script that should only be run on a build server, I don't know 😅, but it kinda shows that having these two scripts was probably a bad idea on my part, since the distinction is subtle and hard to explain.

I've been thinking about solving this already, but never got around to doing it. What do you think of the following:

• We consolidate the two scripts into one generate-css.
• The script will check whether it's running on Heroku or not. If it's running on Heroku, it will do the production variant (with stripping). If not, we assume it's running on a developer machine and do the full build.
• We temporarily change generate-development-css into a script that prints a big warning banner about deprecation, and calls the other script instead (to notify people to move over, without breaking their workflow while they're trying to get something else done). We can then remove it at a later time (say, a month or so).

This should remove the confusion, while still keeping the behavior that we desire.

Would you mind making that change if you agree?

Cheers,
Rico

(*) Of course, the goal itself is up for debate as well, as is the method for achieving it. If you can think of other ways to keep the development experience smooth without committing those files to version control, I'd be happy to talk about those as well.

[reedspool]

Well, I use it all the time

Whoops! I knew this was a baseless assumption but I could have said that better.

if this rationale is not in the contributing guide, we should probably add it

Maybe its woven in, but it's not explicit. Deserves some words I agree - whatever we decide requires an update to the guide.

a developer should only need git and Python, and only need to check out the repository and run python app.py to get up and running

Great goal - makes sense to me. Except that this project uses TypeScript and Tailwind, two things that require build steps for any changes with tools outside of Python. This revision makes perfect sense to me: "you only need Python and git unless you're working on the front-end." But I'm totally biased because these tools are my bread and butter!

the multi-megabyte "full" CSS File to git, so that, while developing, all classes are available

The reason I want to make the above distinction is that within a half-hour of starting to work on a CSS-related issue, I ran into the need to run one of these scripts for myself. I'd added a Tailwind class that wasn't in the committed CSS (gap-0 if you want to verify). So I drew the conclusion that the script, and the node-based dependencies, are requirements for front-end experimentation. Maybe I missed something though!

But, all this might be a moot point if you and the team like the following solution:

Tailwind has an alternative which achieves the stated goal in a different way. It's called the "Play CDN". You simply add a JS script tag to your HTML. Their JS scans the page and then requests an ad-hoc, fit-to-purpose stylesheet from Tailwind's server. The effect is that you get all the benefits of Tailwind's build-step without any dependency. It does require an Internet connection though, which is an important distinction.

Here's a random codepen in which I use it (it's in the JS settings).

The Play CDN is not fit for production use. So to use it, there would need to be a switch in the HTML server (templates/layout.html I think?) which follows this pseudo-code logic:

{% if app.is_in_development_mode %}
<script src="https://cdn.tailwindcss.com"></script>
{% else %}
<link rel="stylesheet" href="{{static('/css/generated.css')}}">
{% endif %}

So this adds two complications as far as I can see: 1. Internet access required and 2. Additional logic (maybe the addition of "dev mode"?) in the HTML server/templates.

And the problem it solves is that the development environment loses this dependency.

Back to your response

having these two scripts was probably a bad idea

I disagree because they each do one thing well. And I don't think consolidating the scripts would be worth the effort in both code and devs adjusting their habits - this confusion can be solved with documentation I think. The two scripts are simple; it's just that their use isn't obvious. The only change I'd recommend (other than updates to the contributing guide) is a name change for generate-css to make it clear it's for deployment.

the goal itself is up for debate as well

It's a clever goal, but not overly clever, I like it. I'd be interested in how successful the concept has been for current and past devs before making any changes. Don't want to make any volunteer's life more difficult, so I wonder who's using which and how often? Any effective way to take a polite poll?

[reedspool]

I did a proof of concept with Play CDN.

main...reedspool:hedy:feat-add-tailwind-dev-play-cdn

You can run this by pulling my branch and simply running app.py - No tailwind generation is needed for development CSS changes. Everything seems to work as far as I can see. There is a "FOUC" Flash of Unstyled Content while the Play CDN does its thing. That is, on every page load, for a moment, everything looks terribly wrong. Then it looks good in the blink of an eye!

There are five file changes in that diff. Here's an explanation:

1. In app.py, simply add the custom jinja2 extension I made
2. Ignore the changes to tailwind.full.js - This was an experiment for using generate-development-css script, but it's not used.
3. generated.css was deleted from the repository to show it's not used anymore (in dev that is. In prod its still needed and layout.html would need to include it again)
4. layout.html first adds a script tag for the Play CDN script. Then it uses the new jinja2 extension to include (raw) the tailwind config JS file as well as styles.css. The contents of both of these files are then used by Tailwind Play CDN to generate a correct stylesheet at page-load
5. The jinja2 extension itself.

The jinja2 extension just includes files literally in the HTML to avoid manually copying and pasting them in there. It shows that no extra work is needed to change the current CSS configuration strategy - simply get it in the right place and the Play CDN does magic.

To make this ready, the pseudocode about a dev-mode switch for Play CDN or generated.css is still needed and missing.

[rix0rrr]

Let me start off by apologizing that I did not have a chance to play with your proposal yet. I'm not a big fan of FOUCs, so if that's what's involved I'm not too keen. For an SPA this might be fine, but Hedy is a good ole'-fashioned page-load-based application and so developers would see the FOUC quite often.

But your idea gave me another idea, which might be even simpler: your idea hinges on emitting different HTML between whether we're in development mode or production mode. Given that we can do that anyway, couldn't we do the following:

{% if app.is_in_development_mode %}
<link rel="stylesheet" href="{{static('/css/generated.full.css')}}">
{% else %}
<link rel="stylesheet" href="{{static('/css/generated.css')}}">
{% endif %}

That is, we'll just have 2 different CSS files, one that's produced by the one script and one that's produced by the other script, and we switch between which one we serve depending on context? If we have that, it becomes impossible to overwrite the right file with the wrong contents.

You would rarely need to run the script anymore anyway. Some cases I can still think of:

• Adding classes to styles.css.
• If you want to add a new prefix-based class (for example hover:bg-blue-300, which by default is not emitted even in the full config because the combinatorial explosion would just grow too large)

I wouldn't necessarily be too sad about these, and I have to admit the simplicity is appealing to me.

[rix0rrr]

(Lots of cleverness in your code btw 🤓, I enjoyed reading that. I like how you're basing it off the current config!)

[reedspool]

Great point about the FOUC - I agree it adds a confusion and burden for everyone, all the time. That outweighs the benefit for the smaller segment of CSS editors.

That's a great idea to split into two different files. I think it clarifies that there are two different processes at work.

I'm confused about the real value of (now) generate.full.css. As I mentioned above, it was missing a basic tailwind class, .gap-0, the first time I used it. That conflicts with how I'm hearing that it should work, namely that it includes "all" tailwind classes. Can you help me straighten this out?

I'm glad you enjoyed my code :) it was fun to explore Flask/Jinja a bit as I'm a Python newbie. And I really didn't want to fill up the PR by copying and pasting that huge Tailwind theme.

Another thing that came out of my experiment is that the two Tailwind config files can be deduped - one can simply import the other one and change only the couple things different.

So the action items so far are

1. Make sure the abstract goal is documented, i.e. "to avoid javascript build tools for the bulk of python-only contributors"
2. Dedupe the tailwind config
3. Split the two CSS files in the CSS scripts
4. Switch in the layout template between the two files according to current environment
5. (Someone please help me) Me understanding what generate.full.css is actually accomplishing

[rix0rrr]

I think what you ran into is that that file should have been the full css, but wasn't because someone else was also confused and ran the prod script instead of the dev script

[reedspool]

Ah you got it. Just verified that main currently has the prod CSS. So now the name change will prevent that ever happening again accidentally. Thank you for clearing that up!

Okay I need help with the layout.html switch. Specifically, what is the real way to do if app.is_in_development_mode? I think once I have that I can take this the rest of the way.

[rix0rrr]

I think we should be able to do something like this:

import utils

if __name__ == '__main__':
  utils.set_debug_mode(True)
  app.add_template_global(utils.is_debug_mode)

And then:

{% if is_debug_mode() %}

Struggling to find the API that would allow us to register a value, instead of a function. I think it would have to be this:

@app.context_processor
def inject_debug_mode_info():
  return {
     "is_debug_mode": utils.is_debug_mode(),
  }

Use whichever you like better.

[Felienne]

Hi @reedspool! I think that can be closed (with #4148), if you agree you can close it!