Add webpack for static asset generation #88

hoetmaaiers · 2020-02-13T07:23:49Z

By adding Webpack to the theme development stack, some tasks become available or easier.

It is possible to write scss that compiles to css. Scss has numorous advantages over vanilla css.
Additional steps will be added to ensure vendor prefixes are added to the css output. This ensures browser compatibility.
With Webpack javascript also can be compiled from modern (e.g. ES2020) into browser compatible javascript.

Via package.json all necessary dependencies can be installed:

yarn install

After installation 2 scripts are available:

yarn build:dev: which spins up a development server and makes the docs demo available on localhost:1919. When changing theme.scss, theme.js or any './docs//*.rst', './docs//*.py' file, the demo at localhost:1919 will be reloaded.
yarn build:production: this generates production ready assets from theme.scss and theme.js into the the directory.

jorisvandenbossche · 2020-02-15T10:03:42Z

So to test this out, I did:

Install yarn (which seems to be in conda-forge)

conda install yarn

Then run from the root of the repo:

yarn install

Then I ran

yarn build:dev

which built the demo docs, I opened this in http://localhost:1919. And if I then made a change in one of the rst files, this automatically triggered to run sphinx again, and I could refresh the rendered html version to see the change.

This seems really nice for working on the theme (as long as the sphinx site doesn't take a long time to build, which is now the case for the demo docs. For the actual pandas docs you would probably not want such automatic trigger, as building it can take a while).

@stijnvanhoey @choldgraf any concerns in adding this?
It certainly adds some tech that most python developers are not familiar with. But I could test it in like 15 min (description above), so it seems rather easy, if we add good contributing guidelines.
As far as I understand, the actual generated css/js files will still be included in the repo, so installing it with pip directly from github master (as projects now do) should still work.

jorisvandenbossche · 2020-02-15T10:05:59Z

@hoetmaaiers running yarn build:production gives an error (I don't know if that is already supposed to work?): "Error: Cannot find module 'optimize-css-assets-webpack-plugin'"

choldgraf · 2020-02-15T10:11:54Z

I can give a shot at the instructions and see how confusing it is. To me the biggest danger is that our core dev audience probably doesn't use yarn. If we use this we should make sure to document well how it works and make sure it isn't unstable for different install environments etc

jorisvandenbossche · 2020-02-15T13:28:58Z

Yes, I fully agree we need to be careful about introducing technical hurdles. Some additional points:

AFAIU, it is only needed when developing the theme, not when using the theme for another project like jupyterhub (which is I think a necessary condition)
I could easily install it with conda (so we can also include it a dev environment yml file), but we should have instructions for non-conda people as well. The instructions at https://yarnpkg.com/getting-started/install are already not very clear I find (it seems to point to an old version of yarn that you need to install with which you can then install a newer version of yarn locally for the project?)
We could also have some make shortcuts for people that are more familiar with that than with yarn. Eg a make html could also ensure to regenerate the css/js files, similarly to what yarn build:dev does.

I can give a shot at the instructions and see how confusing it is

That would be very useful. I think the instructions are basically what I put in #88 (comment)

jorisvandenbossche · 2020-02-15T13:34:04Z

pandas_sphinx_theme/static/css/theme.css

@@ -0,0 +1,387 @@
+/*
+ * nature.css_t


This is the auto-generated file?
If so, can we have a comment at the top saying this?

A comment can be added saying it is autogenerated / only for theme development.

jorisvandenbossche · 2020-02-15T13:36:33Z

pandas_sphinx_theme/static/js/theme.js

@@ -0,0 +1,123 @@
+/******/ (function(modules) { // webpackBootstrap
+/******/ 	// The module cache
+/******/ 	var installedModules = {};


I suppose this is something autogenerated that we don't need to look at?

Correct. This is autogenerated. Or in more frontend terminology, transpiled from modern javascript (which is wat we want to write) into javascript every browser is able to understand, even the older ones.

If it is transpiled every time that the theme builds, then should we avoid committing it to the git repository? Or is it still best-practice to commit it?

(same w/ the yarn.lock file...I'm happy to follow the guidance of others on this, since I'm not a JS dev, but just wanna flag it for discussion)

For this theme.js, I think the big advantage of having it commited into the repo, is that you can still do pip install from the repo (as everybody is now doing, since there isn't an official release yet) and have a working theme without needing to transpile it yourself (and thus install the js toolchain).

This seems an important use case to keep working.

For yarn.lock, less sure.

We might want to add some check (on CI, or a pre-commit hook, ..) to ensure the transpiling was run? (or in the future)

It should indeed be committed.

jorisvandenbossche · 2020-02-15T13:42:16Z

webpack.dev.js

@@ -0,0 +1,31 @@
+const path = require('path');
+const merge = require('webpack-merge');
+const exec = require('child_process').exec;


I think you had this from readthedocs, right? Maybe a short comment giving attribution would be good then

My basis where I started from is indeed from readthedocs, the webpack.common file did change so much only basic boilerplate remains.

Giving attribution is a good idea. Maybe we should do this in the theme dev documentation?

jorisvandenbossche · 2020-02-15T13:54:35Z

@hoetmaaiers some questions that might be related to this:

Should we look at including the (minified) bootstrap css/js in pandas_sphinx_theme/static/ as well using yarn/webpack, instead of loading it from their CDN?
It would be interesting that people using the theme could override a few key elements (like the primary color). I suppose that is something that sass variables could help with? But that means that the when setting such a variable in a project using this theme, they also need to use sass? Or can you use css variables for this without the need to recompile?
(this might rather be for a different issue to discuss)

I think my main concern is not so much the difficulty in using it for developing on this theme (I could set it up quickly), but rather in the needed maintenance of the webpack scripts (although they are not that long)

choldgraf · 2020-02-15T23:02:35Z

Just a quick note on this, I think it is worth noting what benefits we can only get with webpack in order to weight the "pros" against the cons.

For example, I agree wholeheartedly that writing SCSS is cleaner and easier to read/maintain than writing CSS. However, I've actually found this to be fairly straightforward when using the Python libsass package. This will compile SCSS and you can write it to a static file locally as a part of the Sphinx build.

If SCSS is the main thing we get w/ webpack, then I am also happy to just write a little Sphinx extension that auto-runs libsass-python and generates a "regular" CSS file. But maybe there are other major benefits we get as well?

This fixes the yarn build:production script

hoetmaaiers · 2020-02-16T10:49:35Z

@hoetmaaiers some questions that might be related to this:

Should we look at including the (minified) bootstrap css/js in pandas_sphinx_theme/static/ as well using yarn/webpack, instead of loading it from their CDN?

Good idea to indeed include the bootstrap css/js not from a CDN. We could install them as an rpm package. This way yarn instapl handles the installation of all dependency packages.

It would be interesting that people using the theme could override a few key elements (like the primary color). I suppose that is something that sass variables could help with? But that means that the when setting such a variable in a project using this theme, they also need to use sass? Or can you use css variables for this without the need to recompile?
(this might rather be for a different issue to discuss)

Allowing people to override a few key elements is indeed important for the theme to succeed. This can be done in different ways:

via sass variables, with the drawback of not being able to tweak the variables without installing the theme-dev stack (yarn, node)
via css variables. Theses variables would still be available in the webpack output. The only problem these have is they are unsupported in Internet Explorer: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties.

I think my main concern is not so much the difficulty in using it for developing on this theme (I could set it up quickly), but rather in the needed maintenance of the webpack scripts (although they are not that long)

Fair concern about the maintenance cost of Webpack, but the scripts are relatively simple and will not require to much maintenance without adding extra bundling steps. Also webpack is moreless a standard which helps in understanding and supporting it.

Just a quick note on this, I think it is worth noting what benefits we can only get with webpack in order to weight the "pros" against the cons.

For example, I agree wholeheartedly that writing SCSS is cleaner and easier to read/maintain than writing CSS. However, I've actually found this to be fairly straightforward when using the Python libsass package. This will compile SCSS and you can write it to a static file locally as a part of the Sphinx build.

If SCSS is the main thing we get w/ webpack, then I am also happy to just write a little Sphinx extension that auto-runs libsass-python and generates a "regular" CSS file. But maybe there are other major benefits we get as well?

Good idea to make the comparison, libsass-python is also interesting.
I try to list some pros/cons for both approaches:

Webpack Pros

Bundling of scss, modern javascript and other assets into a free to define output format.
Autoprefix css output to ensure all browsers are covered: https://github.com/postcss/autoprefixer
Minify css to reduce file size
With webpack comes the nom ecosystem. We can easily install some decencies this way: bootstrap, normalize.css, ...
Autoreload assets + documentation on save any change in the css, javascript or even .rst and .py files in development. This makes developing the theme way more trivial.

Webpack Cons

More dificultt to install for python developers. Conda assists in this, but the package.json & yarn lock files are pretty unknown I assume?
Needs some configuration in the webpack.config.js files to make it work. Changing it, requires understanding it.

I think the libsass-python solution is quite the opposite regarding pros & cons. It is limited to compiling scss to css; nothing more, nothing less.

For me webpack is in favor because of the easier development thanks to live reload on change.

jorisvandenbossche · 2020-02-16T13:41:38Z

Allowing people to override a few key elements is indeed important for the theme to succeed. This can be done in different ways:

Regarding sass variables vs css variables: although bootstrap uses sass, it seems they also have some things as css variables in its compiled css, so you can still use that as well: https://getbootstrap.com/docs/4.4/getting-started/theming/#css-variables (I didn't in detail, but that might be a way to use sass in this repo, but still allow overriding eg just the primary color with css variable in projects using this theme)

For the webpack pros/cons: I think making use of packages that help with cross-browser / cross-platform compatibility can also be useful, so we need to think less about that when writing css.
And the live-editing-reload seems to be handy as well, the productivity of web developers is also important :)

Using a bunch of external packages for this of course also creates complexity, certainly if you are not familiar with the javascript ecosystem (e.g. I suppose that we will need to update versions in package.json from time to time? Are there tools that can help with this?)

I was thinking that we could also have a "pure python fallback" development mode, that would use libsass-python to compile the css and copy the js file into the correct place, in the make html step, so you could still use the default sphinx-way as well. But I assume that once you start to use some of the other pro's of using webpack (eg autoprefix), that won't be a realistic idea.

jorisvandenbossche · 2020-02-20T22:49:08Z

@choldgraf @stijnvanhoey any further thought on this?
We can maybe start with only using the live reload + scss abilities (which is what I think this PR is doing?) that webpack provide, so it is still easy to go back if it turns out to be a hurdle?

jorisvandenbossche · 2020-02-20T23:08:59Z

In readthedocs, it seems they also integrated webpack into setup.py: https://github.com/readthedocs/sphinx_rtd_theme/blob/master/setup.py

choldgraf · 2020-02-20T23:19:59Z

src/scss/_sphinx-bootstrap.scss

+  /* border-left: 5px solid rgb(238, 238, 238) !important; */
+}
+
+table.field-list th.field-name {


in a future PR I guess some of these could be re-written to leverage the SCSS, yeah?

Absolutely. In a different branch / future PR I am working on refactoring the different css files into a structured scss setup.

choldgraf · 2020-02-20T23:22:25Z

I made a few questions/comments, but I think we could give it a shot and see how it feels for a few months. Then revisit after some time to decide if it truly improves the dev experience, or if users report some problems in their use of the theme related to these changes 👍

stijnvanhoey · 2020-02-21T15:09:34Z

The choice for webpack is fine for me, I do think for this kind of project (focus on front-end theme development) the benefits of the webpack approach are there. Still, we should make sure to add some minimal documentation on how to setup this locally and make sure people are not providing PRs with edited css files...

I was thinking that we could also have a "pure python fallback" development mode, that would use libsass-python to compile the css and copy the js file into the correct place, in the make html step, so you could still use the default sphinx-way as well.

Actually I would try to document/describe 1 way to setup everything instead of multiple alternatives, either written ourselve or maybe @hoetmaaiers can refer to a good/stable setup documentation?

choldgraf · 2020-02-21T17:15:28Z

@stijnvanhoey I agree that we should try sticking with "one obvious way of doing things" and try to make up the difference with clear documentation. If over time that one obvious way is really cumbersome then we can discuss changing it

hoetmaaiers · 2020-02-25T07:51:44Z

I will provide a documentation part which goes into theme development. Setup, how to edit the theme, ... Is this something I add to this PR or can it be done in a later PR?

jorisvandenbossche · 2020-02-25T09:56:15Z

Either way is fine for me. I can imagine it makes your other branches easier to manage if this one gets merged sooner rather than later?

hoetmaaiers · 2020-02-25T10:56:29Z

Indeed, I did my other styling working based on this Webpack setup, just for the added convenience of Webpack watching and compiling. Documentation is important, so if we can merge this branch first, I can immediately focus on providing the documentation first as a follow up branch.

jorisvandenbossche · 2020-02-25T10:57:54Z

Let's do it that way then!

jorisvandenbossche · 2020-02-25T10:58:23Z

Thanks @hoetmaaiers ! Let's see how this goes ;)

jorisvandenbossche reviewed Feb 15, 2020

View reviewed changes

Add webpack for static asset generatino

5afc0d2

hoetmaaiers force-pushed the feature/assets-webpack branch from 0922113 to 5afc0d2 Compare February 16, 2020 10:04

Add optimize-css-assets-webpack-plugin

944e0bd

This fixes the yarn build:production script

choldgraf reviewed Feb 20, 2020

View reviewed changes

jorisvandenbossche merged commit 6b0192b into pydata:master Feb 25, 2020

hoetmaaiers deleted the feature/assets-webpack branch February 25, 2020 10:59

This was referenced Feb 25, 2020

"hi from web pack" #97

Closed

Start making tags or releasing versions #99

Closed

hoetmaaiers mentioned this pull request Feb 26, 2020

Add webpack related theme development documentation #100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add webpack for static asset generation #88

Add webpack for static asset generation #88

hoetmaaiers commented Feb 13, 2020

jorisvandenbossche commented Feb 15, 2020

jorisvandenbossche commented Feb 15, 2020

choldgraf commented Feb 15, 2020

jorisvandenbossche commented Feb 15, 2020 •

edited

Loading

jorisvandenbossche Feb 15, 2020

hoetmaaiers Feb 16, 2020 •

edited

Loading

jorisvandenbossche Feb 15, 2020

hoetmaaiers Feb 16, 2020

choldgraf Feb 20, 2020

choldgraf Feb 20, 2020

jorisvandenbossche Feb 21, 2020

hoetmaaiers Feb 26, 2020

jorisvandenbossche Feb 15, 2020

hoetmaaiers Feb 16, 2020

jorisvandenbossche commented Feb 15, 2020

choldgraf commented Feb 15, 2020

hoetmaaiers commented Feb 16, 2020 •

edited

Loading

jorisvandenbossche commented Feb 16, 2020

jorisvandenbossche commented Feb 20, 2020

jorisvandenbossche commented Feb 20, 2020

choldgraf Feb 20, 2020

hoetmaaiers Feb 25, 2020

choldgraf commented Feb 20, 2020 •

edited

Loading

stijnvanhoey commented Feb 21, 2020

choldgraf commented Feb 21, 2020

hoetmaaiers commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

hoetmaaiers commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

Add webpack for static asset generation #88

Add webpack for static asset generation #88

Conversation

hoetmaaiers commented Feb 13, 2020

jorisvandenbossche commented Feb 15, 2020

jorisvandenbossche commented Feb 15, 2020

choldgraf commented Feb 15, 2020

jorisvandenbossche commented Feb 15, 2020 • edited Loading

Choose a reason for hiding this comment

hoetmaaiers Feb 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Feb 15, 2020

choldgraf commented Feb 15, 2020

hoetmaaiers commented Feb 16, 2020 • edited Loading

jorisvandenbossche commented Feb 16, 2020

jorisvandenbossche commented Feb 20, 2020

jorisvandenbossche commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf commented Feb 20, 2020 • edited Loading

stijnvanhoey commented Feb 21, 2020

choldgraf commented Feb 21, 2020

hoetmaaiers commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

hoetmaaiers commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

jorisvandenbossche commented Feb 25, 2020

jorisvandenbossche commented Feb 15, 2020 •

edited

Loading

hoetmaaiers Feb 16, 2020 •

edited

Loading

hoetmaaiers commented Feb 16, 2020 •

edited

Loading

choldgraf commented Feb 20, 2020 •

edited

Loading