|
| 1 | +0007 Compile Common Theme for all MFEs |
| 2 | +###################################### |
| 3 | + |
| 4 | + |
| 5 | +Status |
| 6 | +****** |
| 7 | + |
| 8 | +Draft |
| 9 | + |
| 10 | +Context |
| 11 | +******* |
| 12 | + |
| 13 | +Currently each Open edX MFE includes a small snippet of SCSS code that is common |
| 14 | +to all of them, which is the following four lines: |
| 15 | + |
| 16 | +.. code-block:: SCSS |
| 17 | +
|
| 18 | + @import "~@edx/brand/paragon/fonts"; |
| 19 | + @import "~@edx/brand/paragon/variables"; |
| 20 | + @import "~@edx/paragon/scss/core/core"; |
| 21 | + @import "~@edx/brand/paragon/overrides"; |
| 22 | +
|
| 23 | +These four lines, import the Paragon and bootstrap code and apply |
| 24 | +branding-specific overrides on top. They contain the bulk of the SCSS code |
| 25 | +generated for MFEs. Following this are usually imports for the frontend header |
| 26 | +and footer components, imports from the MFE own components and MFE-specific |
| 27 | +SCSS. |
| 28 | + |
| 29 | +This effectively means that all MFEs share a large chunk of common styling code |
| 30 | +with a comparatively much smaller set of styles applied over that. |
| 31 | + |
| 32 | +There is scope here to properly split the MFE stylesheet into multiple parts, |
| 33 | +one being the common code as represented by the above imports, and another for |
| 34 | +MFE-specific code, and build them separately. |
| 35 | + |
| 36 | +One thing that prevents this is that MFEs need these SCSS imports to get |
| 37 | +variables. In a number of cases these can be replaced with either |
| 38 | +Paragon/Bootstrap classes, or CSS variables. A separate ADR in `Paragon |
| 39 | +<https://github.com/openedx/paragon/pull/1388>`_ covers the situations where |
| 40 | +this isn't possible yet. |
| 41 | + |
| 42 | +Decision |
| 43 | +******** |
| 44 | + |
| 45 | +Common cores styling that is to be applied to all MFEs should be part of a |
| 46 | +separate stylesheet and built separately from the MFE-specific stylesheet. |
| 47 | + |
| 48 | +Both these stylesheets will be included in each MFE, however the common |
| 49 | +stylesheet can be loaded from a common source for all MFEs. This will allow for |
| 50 | +deploying the common theme stylesheet without redeploying MFEs. |
| 51 | + |
| 52 | +The mechanism for loading this common stylesheet will need to be different from |
| 53 | +the mechanism for loading the MFE stylesheet. A JavaScript theme loader will |
| 54 | +contain the location of the SCSS file and will add this stylesheet to the |
| 55 | +document. |
| 56 | + |
| 57 | +The common theme package will be similar to the branding package, but can be |
| 58 | +deployed independently of MFEs. It will produce a CSS stylesheet, and a |
| 59 | +JavaScript loader that will load this stylesheet. When deploying MFEs, they can |
| 60 | +be provided a configuration variable that contains the location where this |
| 61 | +theme loader is deployed. |
| 62 | + |
| 63 | +The frontend-platform repo can host code that simplifies using themes. It can |
| 64 | +check if a theme loader is configured, and if so, dynamically load the theme |
| 65 | +from there. Otherwise, it can continue using the existing mechanism of building |
| 66 | +and deploying the theme for each MFE for backwards-compatibility. It can also |
| 67 | +provide hooks for loading the theme and potentially/eventually also selecting |
| 68 | +a theme. |
| 69 | + |
| 70 | +Consequences |
| 71 | +************ |
| 72 | + |
| 73 | +With a single common stylesheet for all MFE, they should load quicker since the |
| 74 | +stylesheet can be cached once for all MFEs. It also makes theme rebuilds much |
| 75 | +quicker, and eliminates the need for rebuilding each MFE for minor change to |
| 76 | +the theme. |
| 77 | + |
| 78 | +For context, on a sample development machine, rebuilding an MFE takes roughly |
| 79 | +one minute, of this, on the same machine, around 3-4 seconds is the time it |
| 80 | +takes to build the theme. |
| 81 | + |
| 82 | +By continuing to support the existing theming mechanism, the current mechanism |
| 83 | +for theming can continue working for those who need it. |
| 84 | + |
| 85 | +Since there is a JavaScript-based loader involved, there is potential for |
| 86 | +flashing of unstyled content during loading, however this can be avoided by |
| 87 | +making the theme loader part of the init process so it runs before the app is |
| 88 | +considered ready. |
| 89 | + |
| 90 | +Another major consequence of this change would be that the CSS code for Paragon |
| 91 | +would be loaded using a different mechanism than the JavaScript code of Paragon. |
| 92 | +The CSS code would be compiled into the theme whereas the JS code will be loaded |
| 93 | +directly. This means that if different versions of Paragon are used for an MFE |
| 94 | +vs the common theme, then there is a chance that an incompatible version of the |
| 95 | +CSS could be loaded for the MFE. This will make it all the more important to |
| 96 | +ensure that version of Paragon used across all MFEs is compatible. This will be |
| 97 | +simpler for named releases than for those running latest code changes. |
| 98 | + |
| 99 | +Rejected Alternatives |
| 100 | +********************* |
| 101 | + |
| 102 | +- **Loading the common CSS directly instead of with a JavaScript based loader** |
| 103 | + |
| 104 | + This ADR proposes using an intermediary JavaScript loader to load CSS rather |
| 105 | + then just having the configuration variable point to the stylesheet itself. |
| 106 | + This is done for performance reasons. |
| 107 | + |
| 108 | + Currently, when an MFE is built, each static asset contains the hash of the |
| 109 | + asset in the file name. The ``index.html`` file includes these generated file |
| 110 | + names, and if any asset changes the ``index.html`` file is updated. |
| 111 | + |
| 112 | + When a user reloads an MFE page the index page is reloaded, and if any assets |
| 113 | + have changed, their file names will have as well and they will be loaded |
| 114 | + fresh. Unchanged assets will keep using the same name and can load from the |
| 115 | + cache for a very long time. |
| 116 | + |
| 117 | + If the file names don't change with the content, then there is a risk of |
| 118 | + loading older code from the cache, or (if caching is disabled) of loading a |
| 119 | + huge chunk of code with each refresh and having a huge hit on performance. |
| 120 | + |
| 121 | + So to maximise the amount of time for which stylesheets are cached, and |
| 122 | + minimise the amount of code that needs to be loaded on each refresh, we need |
| 123 | + to have a light intermediary loader that provides the location of the |
| 124 | + stylesheet. The bulk of the CSS can then be cached for as long as it is |
| 125 | + unchanged. |
| 126 | + |
| 127 | +- **Have the theme loading code in each MFE** |
| 128 | + |
| 129 | + This doesn't change much about the rest of the mechanism of this ADR, but it |
| 130 | + causes a lot of code duplication across MFEs. Just like ``frontend-platform`` |
| 131 | + handles authentication for all MFEs, it can also handle dealing with the theme |
| 132 | + loader. |
| 133 | + |
| 134 | + |
| 135 | +References |
| 136 | +********** |
| 137 | + |
| 138 | +- `ADR to introduce CSS variables to parargon |
| 139 | + <https://github.com/openedx/paragon/pull/1388>`_ |
0 commit comments