♻️ REFACTOR: Compile SCSS in pre-commit

[chrisjsewell]

Compiled CSS is now included as part of the distribution, with pre-commit ensuring that the CSS is consistent with the SCSS. This removes pyScss as a dependency, and should mitigate any issues with faulty CSS compilation.

[chrisjsewell]

FYI @choldgraf @pradyunsg @AakashGfude @mmcky, following my musings in #31, I've move away from the "dynamic" SCSS compilation (i.e. during sphinx-build), and instead implemented https://github.com/executablebooks/scss-compile. This allows for a pre-commit compilation of the CSS, which ensures it is synced with the SCSS (and so both are present in the repo). This ticks the boxes:

• No installation dependency on an SCSS compiler, meaning also no potential issues with SCSS compilation for users.
• You don't have to implement an overly "heavy" web-bundler setup, like weppack or gulp
• It still works with tox + sphinx-autobuild, for live development that updates on code/SCSS/docs changes
• Hashed files, to ensure cache invalidation for new CSS
• Later possibility for CSS -> SCSS mapping implementation.

I've also added the (CSS only) tabbed directive 😄, and set up the docs/tox to build for multiple themes in the same branch (see the Development section of the README):

• With sphinx-rtd-theme: https://sphinx-panels--32.org.readthedocs.build/en/32/#tabbed-content
• With sphinx-book-theme: https://106-260360729-gh.circle-artifacts.com/0/html/index.html#tabbed-content

Lastly, this now also includes conf.py configurable CSS variables, via creation of a separate CSS file, that just contains some CSS custom properties. This allows for configuration of certain variables (like tab label color) without the need to create custom CSS overrides.

[pradyunsg]

Wheee! There's lots of cool stuff here, and I like almost all of it.

The bit that concerns me is the use of pre-commit to compile the SCSS.

• It's basically impractical to use a PR based workflow with this approach -- each PR will unconditionally have different generated CSS, so merging any one PR will cause all other existing PRs to conflict with it.
• It raises the minimum required effort to make bug fixes changes from "change the code correctly" to "change the code correctly and regenerate the CSS" which makes it basically super difficult to make GitHub UI only fixes like #33.

I think the first bullet is a deal breaker. :)

[pradyunsg]

I think a good approach here would be to treat the SCSS like a C extension -- include the source code in the source distribution and the compiled form (i.e. CSS) in the generated wheel.

It should be doable already with setuptools (I'd suggest writing a little plugin to make it reusable across packages) and I'm checking how Thomas feels about adding a build step to flit (https://github.com/takluyver/flit/issues/119#issuecomment-687779285).

[pradyunsg]

Maybe this should become a meta issue about "how to handle SCSS", where we discuss and standardize the approach across all the executablebooks projects?

[chrisjsewell]

With all due respect I disagree lol

I think a good approach here would be to treat the SCSS like a C extension -- include the source code in the source distribution and the compiled form (i.e. CSS) in the generated wheel.

This increases the complexity of the package and precludes interactive development workflows

It raises the minimum required effort to make bug fixes changes from "change the code correctly" to "change the code correctly and regenerate the CSS"

If you install pre-commit you don't need to worry about this, it just does it automatically

each PR will unconditionally have different generated CSS, so merging any one PR will cause all other existing PRs to conflict with it

I haven't come across this issue, will have a test now

[chrisjsewell]

I haven't come across this issue, will have a test now

I guess this is what you meant: #35, #36. Basically you always need to merge branches into master which are up-to-date

Hmm, yeh I'll have a play around with compiling during distribution & development only then

[jorisvandenbossche]

I think being able to use a project directly from git (to test the master version) without needing to compile is also valuable.

Regarding the PR workflow: do yo mean that any PR will need to be updated after another PR is merged because of the name change? (so eg you can't merge two PRs directly after each other) (that's a problem for pydata-sphinx-theme then as well ...)

[pradyunsg]

do yo mean that any PR will need to be updated after another PR is merged because of the name change? (so eg you can't merge two PRs directly after each other)

Yup!

[pradyunsg]

I think being able to use a project directly from git (to test the master version) without needing to compile is also valuable.

Indeed.

I think what we have here is a choice among multiple options where each has a different tradeoff, so we should really make an issue somewhere (is meta the right place?) to aggregate "what would be nice to have" as well as all the possible approaches, compare them and make a choice for all our projects based off that. :)

[chrisjsewell]

that's a problem for pydata-sphinx-theme then as well

quite possibly, as I think you are pretty much doing what I'm doing; in terms of creating a hashed CSS file (and deleting any old ones) via a pre-commit, and failing if it does not already exist.

You can easily test these there:

• 35 attempts to merge a branch with master+1 commit into another branch master+1 different commit

• 36 attempts to merge a branch with master-1+1 commits

[chrisjsewell]

is meta the right place?

yep I think so

just to finish my thoughts here, basically as you say each option seems to have an annoying tradeoff. Putting aside JavaScript compilation for now, my ideal goals are:

• Using SCSS as the CSS source, which is linted in the pre-commit

• Having hashed CSS end up in the built documentation

• Having a way of allowing users to configure certain variables (colors/font-size), without having to add their own custom CSS overrides. sphinx allows for these .css_t Jinja templates, but I'm not really a fan of them, because obviously this means now you can't use SCSS and/or lint/compress the CSS. I think the solution I've implemented here works pretty well: dynamic creation of a small additional CSS file, with some custom properties declared in the root namespace.

• Being able to do live development; whereby you can have the documentation in the browser, which updates when your messing around with the any of the Python code, SCSS or RST/Markdown pages

  • This would also be run via tox, so you don't have to set-up development environment manually.
  • If possible it would avoid having to use a nodeJS infrastructure, especially if its just to compile a few SCSS files (although it may be unavoidable if you have more comlex requirements, like pydata-sphinx-theme)

• Not having the SCSS compiler as an install dependency. It's not necessarily a bad option, sphinx after all is a build system; however, the available packages are both not ideal; python-libsass adds a large install overhead, because it has C-bindings, pyScss is lightweight, but not very well maintained, has some bugs, and lacks some key features. Plus it doesn't work great with how sphinx handles theme packages.

• (Having the SCSS and sourcemaps in the built documentation would be nice, but low priority)