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.

Sign up for GitHub

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

Jump to bottom

Revive Tailwind CSS Compilation #246

Merged
merged 28 commits into from Oct 27, 2022
Merged

Revive Tailwind CSS Compilation #246

merged 28 commits into from Oct 27, 2022

Conversation

zampino
Copy link
Collaborator

@zampino zampino commented Oct 26, 2022 •
edited

Workflow

  • opt-in during build static app or (bb task?)
  • run tailwind command against static pages
    • input: stylesheets from resources
    • config: tw config from resources
    • content: html pages + js (build js locally or copy from CAS)
  • check bundle vs unbundled case
  • check with custom viewers producing hiccup/html
  • upload purged CSS to CAS (can we use dejavu ?) copy to out folder
  • tailwind play cdn link need to be replaced with stylesheet link pointing at the CAS URL.

API

If it's meant to be part of build-static-app! we could start by passing an :optimize-css-fn option. The function is responsible for the whole workflow above except for the string replacement, and returns a stylesheet URL. We could handle the replacement on our side.

Current Config

{
   darkMode: \"class\",
   content: [\"./public/build/index.html\", \"./public/build/**/*.html\", \"./build/viewer.js\"],
   safelist: ['dark'],
   theme: {
     extend: {},
     fontFamily: {
       sans: [\"Fira Sans\", \"-apple-system\", \"BlinkMacSystemFont\", \"sans-serif\"],
       serif: [\"PT Serif\", \"serif\"],
       mono: [\"Fira Mono\", \"monospace\"]
     }
   },
   variants: {
     extend: {},
   },
   plugins: [require(\"@tailwindcss/typography\")],
 }

Prior Art

clerk

clerk/src/nextjournal/clerk.clj

Lines 357 to 366 in 914c310

(when purge-css?
(copy-resource! (io/resource "css/tailwind.config.js") "tailwind.config.js")
(let [tailwind-command ["npx" "tailwindcss"
"--config" "tailwind.config.js"
"--output" (str out-path fs/file-separator "css/viewer.css") ;; TODO: check if this works for the bundle? false case
"--minify"
"-" :in (io/input-stream (io/resource "css/viewer.css"))]
{:keys [exit out err]} (apply shell/sh tailwind-command)]
(when-not (zero? exit)
(throw (ex-info (str "error running tailwind: `" (str/join " " tailwind-command) "`") {:command tailwind-command :exit exit :out out :err err}) ))))

nextjournal

https://github.com/nextjournal/nextjournal/blob/5a5f01ccb49a70885e272ef65db1589442345cad/ops/src/nextjournal/ops/build.clj#L25-L46

@zampino zampino changed the title Revive Tailwind JIT Revive Tailwind CSS Compilation Oct 27, 2022
@zampino zampino marked this pull request as ready for review October 27, 2022 15:19
mk
mk reviewed Oct 27, 2022
View reviewed changes
deps.edn Outdated
@@ -25,8 +25,10 @@
org.babashka/cli {:mvn/version "0.5.40"}}
:extra-paths ["notebooks"]
:exec-fn nextjournal.clerk/build!
:exec-args {:paths-fn nextjournal.clerk.builder/clerk-docs}
:exec-args {:paths-fn nextjournal.clerk.builder/clerk-docs
:compile-css? true}
Copy link
Member

Choose a reason for hiding this comment

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

Drop the ? here please, (like we did for bundle) we normalize this early

src/nextjournal/clerk/builder.clj Outdated
@@ -78,6 +81,7 @@
(str "Errored in " duration ". ❌\n")
(str "Done in " duration ". ✅\n"))
:building (str "🔨 Building \"" (:file doc) "\"… ")
:compiling-css "🎨 Optimizing CSS… "
Copy link
Member

Choose a reason for hiding this comment

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

Compiling Tailwind CSS…

src/nextjournal/clerk/builder.clj Outdated
[{:keys [out-path]} docs]
(assert (and (= 0 (:exit (sh "which" "npx"))) (= 0 (:exit (sh "npx" "tailwindcss"))))
"Clerk's CSS optimizaiton failed: node and tailwind need to be installed. Please run `npm install -D tailwindcss @tailwindcss/typography` and retry.")
(spit "tailwind.config.cjs" (slurp (io/resource "stylesheets/tailwind.config.js")))
Copy link
Member

Choose a reason for hiding this comment

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

Why .cjs and not .js?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because of type: module in the package.json. All .js files are treated like ES modules unless they have a .cjs extension.

src/nextjournal/clerk/builder.clj Outdated
"--config" "tailwind.config.cjs"
"--output" (str (fs/path out-path "viewer.css"))
"--minify")
(swap! config/!resource->url assoc "/css/viewer.css" "viewer.css"))
Copy link
Member

Choose a reason for hiding this comment

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

We should content address the css (like we do for the images) so the file name should be sha512.css

src/nextjournal/clerk/view.clj
(defn include-viewer-css []
(defn relative? [url]
(and (not (.isAbsolute (URI. url)))
(not (str/starts-with? url "/"))))
Copy link
Member

Choose a reason for hiding this comment

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

Why are both of these checks needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because (= false (.isAbsolute (URI. "/viewer.css")))

@zampino zampino merged commit 4c17b0f into main Oct 27, 2022
@zampino zampino deleted the tailwind-jit-II branch October 27, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mk mk mk left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zampino @mk