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

feat: use tailwind binary to build CSS #565

Merged
merged 5 commits into from
Aug 2, 2023
Merged

Conversation

shadows-withal
Copy link
Contributor

Removes the dependency on a Node toolchain + Yarn to build CSS, as long as you're not running a release build. If you're running a release build, nothing changes, we still fetch the associated (or custom) CSS version from GitHub.

Also adds a new hidden command to generate just the CSS for the purposes of attaching it to a release, and modifies the corresponding workflow.

@SaraVieira
Copy link
Contributor

The hero we needed

@@ -52,7 +52,6 @@ module.exports = {
plugins: [
require("@tailwindcss/typography"),
require("@tailwindcss/forms"),
require("@tailwindcss/line-clamp"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(this is a default plugin in tailwind now)

pub fn place_css(dist_dir: &str, release_tag: &str) -> Result<()> {
// Even if you're running a development build, we still respect the custom CSS version preference
// by falling back to fetching said version from GitHub.
if cfg!(debug_assertions) && release_tag == ORANDA_CSS_TAG {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to my knowledge there isn't a better way to find out whether we're running a dev build or a release build, right?

Copy link
Member

Choose a reason for hiding this comment

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

It's either this or cfg(test) (neither of which would work for my habit of developing with --release). This is fine enough.

"https://github.com/tailwindlabs/tailwindcss/releases/latest/download/tailwindcss-{double}"
);
let handle = tokio::runtime::Handle::current();
let response = handle.block_on(reqwest::get(url))?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI: axoasset currently doesn't handle requests for a binary correctly, so for now I'm falling back to using reqwest (which is already in our dependency tree)

Copy link
Member

Choose a reason for hiding this comment

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

Oh because it insists everything is utf8 text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, because it does very naive mime type checking which basically only accounts for images, so it gets fussy and quits

Copy link
Member

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

Seeeems reasonable?

("windows", "aarch64") => "windows-arm64.exe",
_ => "linux-x64",
};
let mut binary_path = Utf8PathBuf::from(cache_dir.display().to_string());
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
let mut binary_path = Utf8PathBuf::from(cache_dir.display().to_string());
let mut binary_path = Utf8PathBuf::from_path(cache_dir)?;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesn't work, Utf8PathBuf only has from_path_buf, which is annoyingly a side-effectful operation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Utf8Path has from_path, but we need an owned path)

src/site/layout/css.rs Outdated Show resolved Hide resolved
src/site/layout/css.rs Show resolved Hide resolved
@shadows-withal shadows-withal merged commit a547f4a into main Aug 2, 2023
7 checks passed
@shadows-withal shadows-withal deleted the feat/drop-yarn branch August 2, 2023 13:59
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

3 participants