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

Use Vite as build tool #52

Merged
merged 10 commits into from
Mar 6, 2023
Merged

Use Vite as build tool #52

merged 10 commits into from
Mar 6, 2023

Conversation

jcbhmr
Copy link
Contributor

@jcbhmr jcbhmr commented Feb 1, 2023

This PR would...

  • Use Vite as the build tool
  • NOT change anything about the CSS styles
  • NOT change anything about the URLs/imports that are exposed
  • Bundle all fonts in with the style.min.css as a side-effect of using Vite

Why this is a good idea

https://vitejs.dev/guide/features.html#css

Vite is really good. Like really good in terms of developer experience, tooling support, popularity, etc. It seems to be the absolute no-brainer option for bundling, building, etc. today. This may change in the future, and if it does I encourage you to move to whatever the best thing becomes. But right now I think this is a good build tool to migrate too.

Here's some stats from the State of JS 2022 survey to back up that Vite is on top right now:

image
image

Vite even supports SASS, SCSS, @import-bundling, PostCSS, etc. out of the box. This would open this project up to using SASS or SCSS, @import(...)-bundling, etc. that all compiles to the exact same CSS that we have now! 😎

If you don't sepecify files: [...], then npm will use the .gitignore
which we don't want since it would ignore the dist folder
which we WANT included in the final .tar.gz package.

And, since we specify the files: [...] package.json entry, we must
explicitly list all files/folders that we want in there.
Right now we expect the final thing to be in ./style.min.css, so we need to cp it
there in our build step.
@vercel
Copy link

vercel bot commented Feb 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
latex-css ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 6, 2023 at 7:11AM (UTC)

@vincentdoerig
Copy link
Owner

Thank you for the great idea! The only problem I see right now is that the minimised file (https://latex.vercel.app/style.min.css) is not accessible in your version. If there is a way to make this work, I'll gladly merge this PR:).

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Feb 20, 2023

Oops! I forgot to make sure that it worked with Vercel. 🤦‍♂️ Vercel looks for the npm run build script in package.json, so I needed to add that, and then it worked! 🎉

https://latex-css-git-fork-jcbhmr-use-vite-vincentdoerig.vercel.app/style.min.css

Here is a screenshot of the above ⬆ URL:

image

Does this fix it?

@jcbhmr jcbhmr closed this Feb 27, 2023
@vincentdoerig
Copy link
Owner

Hey, do you mind reopening this PR? I had plans on merging this one, but wanted to get through with the others to avoid any merging conflicts. I apologise for the lack of communication and want to thank you for the meaningful contribution:).

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Feb 27, 2023

Sure! ♥️ Sorry about that.

I had just figured that is continue with the fork on my own to avoid spamming you with PR after PR that would increasingly become based on each other. You can check out some of my changes and additions https://github.com/jcbhmr/latexlike

The big one so far is the storybook website example showcase instead of the demo pages. https://jcbhmr.github.io/latexlike/?path=/docs/blockquote--alone

@jcbhmr jcbhmr reopened this Feb 27, 2023
@vincentdoerig
Copy link
Owner

Thanks again for the PR, I apologise again for the delays... One last thing I noticed is that we probably should not inline all the fonts into the minified file, since the resulting file size of style.min.css is 2.9MB (!). We should either restrict ourselves to only inlining the regular (and maybe the bold) font file or drop them all together and reference them by the @font-face rule like before (and as it's done in style.css). Thoughts?

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Mar 6, 2023

we probably should not inline all the fonts into the minified file

Good catch! I didn't even think about that. 🤦‍♂️ We could use something like https://github.com/ManBearTM/vite-plugin-no-bundle/#readme and just copy over all the font files. Probably. I haven't tried this yet, but I think this is the solution.

Sitenote: do you know if Chrome or Firefox will always eagerly load fonts if they aren't used (like if I define @font-face for "My Unused Font"), or do they load them on-demand when a font gets used (like font-family: "My Actually Used Font";)? Does font loading block rendering?

You could also require "Load one of the following fonts with your own font-loading mechanism, and we'll automatically use that" though that would be a breaking change and require more justification than just "it was too hard to do what we do now with Vite".

<link rel="stylesheet" href="https://unpkg.com/panduck-font-libertinus" />
<link rel="stylesheet" href="https://unpkg.com/latex.css" />

@jcbhmr jcbhmr deleted the use-vite branch March 6, 2023 20:47
@vincentdoerig
Copy link
Owner

Yes, so I would personally avoid introducing a breaking change.
I am pretty sure that the browser only downloads the fonts it needs (i.e. are being actively referenced in a class used on the page), so the best solution would be to get rid of the inlined fonts in the minified file and replace them with the font-face declarations like in the unminified stylesheet. I will look into how to do this with Vite/the plugin you referenced.

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