Skip to content

Commit

Permalink
build: Remove unneccessary built-in XBlock Sass built steps (#35833)
Browse files Browse the repository at this point in the history 
Since all built-in block Sass is gone, we remove two final pieces of code:

* the steps in `npm run compile-sass` which compiled `xmodule/assets`, and
* the now-unused `add_sass_to_fragment` function.

This should speed up the edx-platform assets build a little bit.
This commit also includes some minor dev doc updates.

Part of: #35300
  • Loading branch information
@kdmccormick
kdmccormick authored Nov 14, 2024
1 parent 5f5b8aa commit 2769a00
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 105 deletions.
52 changes: 0 additions & 52 deletions scripts/compile_sass.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,18 +352,6 @@ def compile_sass_dir(
repo / "lms" / "static" / "sass",
],
)
compile_sass_dir(
"Compiling built-in XBlock Sass for default LMS",
repo / "xmodule" / "assets",
repo / "lms" / "static" / "css",
includes=[
*common_includes,
repo / "lms" / "static" / "sass" / "partials",
repo / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass",
repo / "cms" / "static" / "sass",
],
)
if not skip_cms:
compile_sass_dir(
"Compiling default CMS Sass",
Expand All @@ -376,18 +364,6 @@ def compile_sass_dir(
repo / "cms" / "static" / "sass",
],
)
compile_sass_dir(
"Compiling built-in XBlock Sass for default CMS",
repo / "xmodule" / "assets",
repo / "cms" / "static" / "css",
includes=[
*common_includes,
repo / "lms" / "static" / "sass" / "partials",
repo / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass",
repo / "cms" / "static" / "sass",
],
)
click.secho(f"Done compiling default Sass!", fg="cyan", bold=True)
click.echo()

Expand Down Expand Up @@ -429,20 +405,6 @@ def compile_sass_dir(
],
tolerate_missing=True,
)
compile_sass_dir(
"Compiling built-in XBlock Sass for themed LMS",
repo / "xmodule" / "assets",
theme / "lms" / "static" / "css",
includes=[
*common_includes,
theme / "lms" / "static" / "sass" / "partials",
theme / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass" / "partials",
repo / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass",
repo / "cms" / "static" / "sass",
],
)
if not skip_cms:
compile_sass_dir(
"Compiling default CMS Sass with themed partials",
Expand Down Expand Up @@ -470,20 +432,6 @@ def compile_sass_dir(
],
tolerate_missing=True,
)
compile_sass_dir(
"Compiling built-in XBlock Sass for themed CMS",
repo / "xmodule" / "assets",
theme / "cms" / "static" / "css",
includes=[
*common_includes,
theme / "lms" / "static" / "sass" / "partials",
theme / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass" / "partials",
repo / "cms" / "static" / "sass" / "partials",
repo / "lms" / "static" / "sass",
repo / "cms" / "static" / "sass",
],
)
click.secho(f"Done compiling Sass for theme at {theme}!", fg="cyan", bold=True)
click.echo()

Expand Down
28 changes: 9 additions & 19 deletions xmodule/assets/README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,15 @@ However, we are proactively working towards a system where:
Themable Sass (.scss)
*********************

XBlock CSS for ``student_view``, ``author_view``, and ``public_view`` is compiled from the various ``./<ClassName>BlockDisplay.scss`` modules, such as `AnnotatableBlockDisplay.scss`_.

XBlock CSS for ``studio_view`` is compiled from the various ``./<ClassName>BlockEditor.scss`` modules, such as `AnnotatableBlockEditor.scss`_.

Those Sass modules are mostly thin wrappers around the underscore-prefixed Sass
modules within block-type-subdirectories, such as `annotatable/_display.css`. In the
future, we may `simplify things`_ by collapsing the top-level Sass modules and
just directly compiling the block-type-subdirectories' Sass.

The CSS is compiled into the static folders of both LMS and CMS, as well as into
the corresponding folders in any enabled themes, as part of the edx-platform build.
It is collected into the static root, and then linked to from XBlock fragments by the
``add_sass_to_fragment`` function in `builtin_assets.py`_.

.. _AnnotatableBlockDisplay.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss
.. _AnnotatableBlockEditor.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss
.. _annotatable/_display.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/annotatable/_display.scss
.. _simplify things: https://github.com/openedx/edx-platform/issues/32621

Formerly, built-in XBlock CSS for ``student_view``, ``author_view``, and
``public_view`` was compiled from the various
``./<ClassName>BlockDisplay.scss`` modules, and ``studio_view`` CSS was
compiled from the various ``./<ClassName>BlockEditor.scss`` modules.

As of November 2024, all that built-in XBlock Sass was been permanently
compiled into CSS, stored at ``../static/css-builtin-blocks/``.
The theme-overridable Sass variables are injected into CSS variables via
``../../common/static/sass/_builtin-block-variables.scss``.

JavaScript (.js)
****************
Expand Down
35 changes: 1 addition & 34 deletions xmodule/util/builtin_assets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,47 +32,14 @@ def add_css_to_fragment(fragment, css_relative_path):
if not css_absolute_path.is_file():
raise FileNotFoundError(f"css file not found: {css_absolute_path}")
css_static_path = Path('css-builtin-blocks') / css_relative_path.with_suffix('.css')
css_url = get_static_file_url(str(css_static_path)) # get_static_file_url is theme-aware.
css_url = get_static_file_url(str(css_static_path))
if not css_url:
raise ImproperlyConfigured(
f"Did not find static CSS file {css_static_path} in staticfiles storage. Perhaps it wasn't collected?"
)
fragment.add_css_url(css_url)


def add_sass_to_fragment(fragment, sass_relative_path):
"""
Given a Sass path relative to xmodule/assets, add a compiled CSS URL to the fragment.
Raises:
* ValueError if {sass_relative_path} is absolute or does not end in '.scss'.
* FileNotFoundError if edx-platform/xmodule/assets/{sass_relative_path} is missing.
* ImproperlyConfigured if the lookup of the static CSS URL fails. This could happen
if Sass wasn't compiled, CSS wasn't collected, or the staticfiles app is misconfigured.
Notes:
* This function is theme-aware. That is: If a theme is enabled which provides a compiled
CSS file of the same name, then that CSS file will be used instead.
"""
if not isinstance(sass_relative_path, Path):
sass_relative_path = Path(sass_relative_path)
if sass_relative_path.is_absolute():
raise ValueError(f"sass_relative_path should be relative; is absolute: {sass_relative_path}")
if sass_relative_path.suffix != '.scss':
raise ValueError(f"sass_relative_path should be .scss file; is: {sass_relative_path}")
sass_absolute_path = Path(settings.REPO_ROOT) / "xmodule" / "assets" / sass_relative_path
if not sass_absolute_path.is_file():
raise FileNotFoundError(f"Sass not found: {sass_absolute_path}")
css_static_path = Path('css') / sass_relative_path.with_suffix('.css')
css_url = get_static_file_url(str(css_static_path)) # get_static_file_url is theme-aware.
if not css_url:
raise ImproperlyConfigured(
f"Did not find CSS file {css_static_path} (compiled from {sass_absolute_path}) "
f"in staticfiles storage. Perhaps it wasn't collected?"
)
fragment.add_css_url(css_url)


def add_webpack_js_to_fragment(fragment, bundle_name):
"""
Add all JS webpack chunks to the supplied fragment.
Expand Down

0 comments on commit 2769a00

Please sign in to comment.