Skip to content

Stop bundling the compiled CSS/JS with our theme by following Furo's build pattern #503

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

Closed
2 tasks
choldgraf opened this issue Oct 19, 2021 · 14 comments · Fixed by #514
Closed
2 tasks

Stop bundling the compiled CSS/JS with our theme by following Furo's build pattern #503

choldgraf opened this issue Oct 19, 2021 · 14 comments · Fixed by #514
Assignees

Comments

@choldgraf
Copy link
Collaborator

Description / Summary

Our current setup commits the compiled CSS + hashes directly into our repository. The biggest downside to this is that merge conflicts happen any time somebody merges a PR when another person has a PR open that also modifies CSS or JS.

I saw a pattern from @pradyunsg's Furo theme that I quite liked, so wanted to write it down and suggest it here. The way Furo is set up:

Value / benefit

I think there are two benefits that this workflow would have:

  1. It would reduce the amount of errors that are generated as a result of PRs merging and stepping on each other
  2. It would reduce the committed blobs of non-human-readable text in our repository, which would make the repository a bit easier to follow

Implementation details

One potential downside here is that it introduces the possibility that those assets aren't bundled with the theme when the package is built for distribution. However I think that since we're using GitHub Actions for this anyway, we could add in a step that would ensure that compilation happens before building.

Tasks to complete

  • Are there any objections to trying out this pattern here?
  • If not, give an implementation a try
@jorisvandenbossche
Copy link
Member

With our current setup, I think the main disadvantage would be that you would no longer be able to install directly from master (which is what we still do in pandas .. Maybe I should start using released versions there, but I did that in the beginning to directly dogfood the theme).
Now, if we move away from using webpack (which we probably want to do anyway) to something that is pip installable, that would solve this concern.

@pradyunsg
Copy link
Contributor

pradyunsg commented Oct 23, 2021

  • Q: Are you opposed to moving files around, to fit a build system -- if that also handled the builds for SASS and JS?
  • Q: How strongly do folks feel about moving away from needing JS tooling working (npm and node basically), if you instead had a pure-Python wrapper on top of the NodeJS tooling that gave you clearer error messages about NodeJS misconfiguration?

@choldgraf
Copy link
Collaborator Author

choldgraf commented Oct 23, 2021

Are you opposed to moving files around, to fit a build system

I'm not quite sure what this means, but I don't think I'm opposed to it?

How strongly do folks feel about moving away from needing JS tooling

I think the question to ask is what kind of value we get out of NPM style workflows, and what price do we pay for that value, and where else can we get the same value for a potentially lower price.

I think the main value we get out of npm is vendoring bootstrap and fontawesome, and compiling our SCSS and JS for use in the theme. The price we pay for that is a non-standard development stack (for python developers) that has introduced (in my opinion) a decent amount of barriers to maintaing the theme (extra install step, non-standard tooling, slowness with npm builds, non-standard config files, occasional confusion with yarn lock files, etc).

With web-compile it seems like we have basically the same value but at a much lower cost because it would "just work" if you pip install this package w the dev requirements. We'd have a.new config file to keep track of, but this would be a relatively simple YAML file rather than JSON + a bunch of JavaScript.

So to me it seems like an easy choice for a python-native option that doesn't rely on installing NPM, unless there's a thing that NPM does that we can't do in python only tooling. (I'm open to that argument! Just don't see it right now)

@pradyunsg
Copy link
Contributor

pradyunsg commented Oct 25, 2021

FWIW, those questions are user-research question from my PoV -- as someone writing a PEP 517/PEP 660 (pyproject.toml based) build-backend for Sphinx themes, and is hoping that sphinx-book-theme and pydata-sphinx-theme can be users of that backend. :)

@choldgraf
Copy link
Collaborator Author

choldgraf commented Oct 25, 2021

Just a quick point of info that may be relevant to the web-compile conversation, libsass is deprecated and the libsass-python maintainer doesn't seem to plan on updating to dart-sass (which is the recommended new sass library)

@jorisvandenbossche
Copy link
Member

@pradyunsg to both of your questions, I think my answer would be: "I don't care, as long as we have a workflow the maintainers are happy with and we serve our users (eg installing from source still works)"
Well, I do care about having happy contributors and thus an easy to use workflow, but I don't strongly care about file layout :). And I do care about shared tooling and workflows (so the custom webpack approach we now have is not ideal given that none of the related packages are using it), so having a build backend that can help with this sounds interesting!

Do you have a bit more details already on what you are thinking? (AFAIK for furo you are also using js tooling, although not webpack as we currently do here)

@pradyunsg
Copy link
Contributor

pradyunsg commented Oct 27, 2021

Do you have a bit more details already on what you are thinking?

Yes! Basically, I wanna make it easier to use NodeJS-based tooling for writing Sphinx themes, with additional development workflow niceties tacked on to push for a couple of things I feel would be beneficial.

  1. No checking-in compiled artifacts into the repository. (i.e. this issue)
  2. Be pip installable from the git checkout directly. (Furo is not)
  3. Provide a "sane default" scafolding with a decent build pipeline, that theme authors can customise / improve / replace easily or adapt their existing theme into it easily.
  4. Enforce a package structure + expected output filenames, which enables more consistency across projects (i.e. easier to contribute, more transferable understanding) and allows the build processes to be more streamlined.
  5. Provide a generally nice workflow-helper CLI, which utilises the shared structure to enforce similar workflows.

Notably, I don't think this would help with getting rid of the custom webpack pipeline. Instead, it should help alleviate some of the workflow pains/quirks that arise from needing that build pipeline for primarily-familiar-with-Python-and-not-JS contributors, and enforce some expectations on the output of that pipeline to make it easier to contribute across projects.

I might explore how to make maintaining/authoring the JS build pipelines easier, but... not now.


Oh, and... I got 3/4 commands working! (the stb new command is the last bit)

Screenshot showing `stb --help` and the output of `stb compile`, @pradyunsg apologies to screen reader users for using this

@pradyunsg
Copy link
Contributor

pradyunsg commented Oct 27, 2021

Basically, this wraps nodeenv (https://pypi.org/project/nodeenv/), running npm run-script build within that nodeenv, https://github.com/pypa/build and https://github.com/executablebooks/sphinx-autobuild/ into a single package that is easier to reason about. :)

@pradyunsg
Copy link
Contributor

Alrighty. I've made significant process on this and switched Furo over to https://github.com/pradyunsg/sphinx-theme-builder. You can pip install git+https://github.com/pradyunsg/furo/ and it should just work!

pradyunsg/furo@e62e08d is the commit with most of the stuff; although it took me a few tries to get things working. :)

I still need to do the documentation (the getting started guide is out of date now) and tests on that. I've also started the work of porting over this theme, as a proof of concept. I'll clean up some of the story around the webpack configuration as well, so... uhm... don't touch that for a bit! :)

@pradyunsg
Copy link
Contributor

I'll file a PR today evening, after polishing a couple more things.

@jorisvandenbossche
Copy link
Member

Cool! I will try to play with that a bit tomorrow or the coming days to provide feedback.

(and sorry I forgot to answer on your previous comment / screenshot, but so that's certainly looking interesting :))

@pradyunsg
Copy link
Contributor

Neato! No hurries on this from my end. :)

@choldgraf
Copy link
Collaborator Author

@jorisvandenbossche just FYI I gave a shot at doing this for a sub-theme that I have and it worked really nicely. Here is the PR for the changes: 2i2c-org/sphinx-2i2c-theme#2

@pradyunsg
Copy link
Contributor

#514 is ready for review!

@damianavila damianavila moved this to Todo 👍 in Sprint Board Nov 26, 2021
@damianavila damianavila self-assigned this Nov 26, 2021
@damianavila damianavila moved this from Todo 👍 to In Progress ⚡ in Sprint Board Nov 29, 2021
Repository owner moved this from In Progress ⚡ to Done 🎉 in Sprint Board Dec 3, 2021
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 a pull request may close this issue.

4 participants