Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Kdmccormick/xmodule assets #32272

Closed
wants to merge 11 commits into from
Closed

Kdmccormick/xmodule assets #32272

wants to merge 11 commits into from

Commits on May 22, 2023

  1. refactor: define resource paths (not contents) on XModule classes 

    For the XBlocks types that use legacy XModule-style assets, this is small
refactor that brings them a bit closer to being like XBlocks.

Given class attributes on those block types in the form:

    SomeXModuleLikeBlock.(studio|preview)_view_(js|css)['(js|scss|css|xmodule_js)']

we make it so their value is the *path to the resource*
rather than *the actual content of the resource*.

This will make future refactorings simpler.

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    ece1dbd View commit details
    Browse the repository at this point in the history

  2. build: stop prefixing copied XModule SCSS with indexes 

    The `xmodule_assets` command copies SCSS files from
xmodule/css to common/static/xmodule/{modules|descriptors}/scss.
It renames the files to the format:

   _{INDEX}-{HASH}.scss

where an XModule's first SCSS resource will have INDEX==0,
the next will have INDEX==1, ...and that's it because no
XModule has more than two SCSS resources.

The output looks like this:

  common/static/xmodule/descriptors/scss:
    _000-808fcbb4c5109c5156ae3c0c9729c8be.scss
    _000-9bdcda00f046f78be79aca7791e1d4fb.scss
    _000-d41921b4c5d45188759ef3d04fd9a78a.scss
    _001-901b985e5ea2dea2a89cce747cf4307d.scss
    _001-a10fc3e0fd6aca63426a89e75fe69c31.scss
  common/static/xmodule/modules/scss:
    _000-1ad2f05db822d3176affd203d70319c0.scss
    _000-1dc4276d3849a14ea538286e97740c14.scss
    _000-29baf1ef1af89b1051362f51124abd01.scss
    _000-6bf8c2340b013d835b25df13e03b8d33.scss
    _000-8b6bb50b058d34efefa40107307a32c6.scss
    _000-958d6ef6baa09be94bccaf488861c8e5.scss
    _000-a3c2cdf2141d24a76be9afa56f237c29.scss
    _000-b80300e1a5f290f6a850e35874068427.scss
    _001-482ebc752ab6e41946651ceb0f3e7f55.scss

These indexes serve no purpose. Reading the comments
and git-blame in xmodule/static_content.py, one can glean
that they indexes might have been intended to enforce
dependency relationships between the assets, but
this is unnecessary, because the ordering of the copied
SCSS is *already preserved* by the order which they're
included into the {BLOCK_NAME}{Studio|Preivew}.{HASH}.scss
SCSS entrypoint files. I have to assume that this is an
unnecessary relic from the timebefore the XModule system was
reduced to a legacy corner of the XBlock framework.

So, we remove the indexes, which lets us simplify the logic
of xmodule/static_content.py. This is a minor refactoring, but it'll
make it easier for the next steps on our way to deleting
xmodule/static_content.py entirely. The new output looks like this:

  common/static/xmodule/descriptors/scss:
    _808fcbb4c5109c5156ae3c0c9729c8be.scss
    _901b985e5ea2dea2a89cce747cf4307d.scss
    _9bdcda00f046f78be79aca7791e1d4fb.scss
    _a10fc3e0fd6aca63426a89e75fe69c31.scss
    _d41921b4c5d45188759ef3d04fd9a78a.scss
  common/static/xmodule/modules/scss:
    _1ad2f05db822d3176affd203d70319c0.scss
    _1dc4276d3849a14ea538286e97740c14.scss
    _29baf1ef1af89b1051362f51124abd01.scss
    _482ebc752ab6e41946651ceb0f3e7f55.scss
    _6bf8c2340b013d835b25df13e03b8d33.scss
    _8b6bb50b058d34efefa40107307a32c6.scss
    _958d6ef6baa09be94bccaf488861c8e5.scss
    _a3c2cdf2141d24a76be9afa56f237c29.scss
    _b80300e1a5f290f6a850e35874068427.scss

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    a5e1e8c View commit details
    Browse the repository at this point in the history

  3. build: stop suffixing XModule SCSS with hashes 

    Similar to the previous commit, this is another thing that
xmodule_assets does to its generated output that is completely
unnecessary. The XModule SCSS entrypoint files are suffixed with
MD5 hashes:

  common/static/xmodule/descriptors/scss:
     AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
     AnnotatableBlockStudio.be69909d83985d31e206fad272906958.scss
     ConditionalBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     CourseInfoBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
     CustomTagBlockStudio.be69909d83985d31e206fad272906958.scss
     HtmlBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
     LibraryContentBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     LTIBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     PollBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     ProblemBlockStudio.5893b30426f88e14712556c6c4342f23.scss
     SequenceBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     SplitTestBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
     StaticTabBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
     VideoBlockStudio.e4a6920a875dfb91eb65ee7e6dad7e2e.scss
     WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
  common/static/xmodule/modules/scss:
     AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
     AnnotatableBlockPreview.7e95b106aa0a61824f4290da1374960d.scss
     ConditionalBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
     CourseInfoBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
     CustomTagBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
     HtmlBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
     LibraryContentBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
     LTIBlockPreview.a763928b2c415251720f8634b8daee59.scss
     PollBlockPreview.39730e54c1eebbafd18a82fbb09c1e37.scss
     ProblemBlockPreview.70b905ac161108a0a03c639232450aaa.scss
     SequenceBlockPreview.e2336fa64ba495fa7c0f4f838d20ad8c.scss
     SplitTestBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
     StaticTabBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
     VideoBlockPreview.b2de5e1c4da8cba16ce1c4bdeab50f9e.scss
     WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss

A comment in xmodule/static_content.py hints that it might have
something to do with de-duplication, but that doesn't make any sense,
because each XModule has exactly two entrypoint files, and they aren't
shared.

So, we remove the hashes, as an intermediate step towards deleting
xmodule/static_content.py and instead just checking these SCSS
entrypoints directly into the repo. Until then, the new output
looks like this:

  common/static/xmodule/descriptors:
     AboutBlockStudio.scss
     AnnotatableBlockStudio.scss
     ConditionalBlockStudio.scss
     CourseInfoBlockStudio.scss
     CustomTagBlockStudio.scss
     HtmlBlockStudio.scss
     LibraryContentBlockStudio.scss
     LTIBlockStudio.scss
     PollBlockStudio.scss
     ProblemBlockStudio.scss
     SequenceBlockStudio.scss
     SplitTestBlockStudio.scss
     StaticTabBlockStudio.scss
     VideoBlockStudio.scss
     WordCloudBlockStudio.scss
  common/static/xmodule/modules:
     AboutBlockPreview.scss
     AnnotatableBlockPreview.scss
     ConditionalBlockPreview.scss
     CourseInfoBlockPreview.scss
     CustomTagBlockPreview.scss
     HtmlBlockPreview.scss
     LibraryContentBlockPreview.scss
     LTIBlockPreview.scss
     PollBlockPreview.scss
     ProblemBlockPreview.scss
     SequenceBlockPreview.scss
     SplitTestBlockPreview.scss
     StaticTabBlockPreview.scss
     VideoBlockPreview.scss
     WordCloudBlockPreview.scss

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    9dd7460 View commit details
    Browse the repository at this point in the history

  4. build: import XModule source SCSS directly rather than copying 

    The xmodule_assets command copies SCSS source files from
xmodule/css to common/static/xmodule/scss, renaming them
to {MD5_HASH}.scss to "remove duplicates" (this is completely
unnecessary; there are a couple dozen SCSS files and none
of them are duplicates). The copied files are then included
into generated SCSS entry files (eg AnnotatableBlockStudio.scss).

Rather than copying the original SCSS files, we change
the generate SCSS entrypoint files to simply import the
original SCSS files.

For example, common/static/xmodule/descriptors/scss/AboutBlockStudio.scss
is changed from:

    .xmodule_edit.xmodule_AboutBlock {
      @import "9bdcda00f046f78be79aca7791e1d4fb.scss";
      @import "a10fc3e0fd6aca63426a89e75fe69c31.scss";
    }

to:

    .xmodule_edit.xmodule_AboutBlock {
      @import "editor/edit.scss";
      @import "html/edit.scss";
    }

In order to make the @imports work, we add xmodule/css to the list
of lookup dirs for XModule SCSS compilation. Soon, we will
move the SCSS entry files to the same directory as the original
SCSS files, which will allow us to take xmodule/css back out of the list
of SCSS lookup dirs.

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    03b0d81 View commit details
    Browse the repository at this point in the history

  5. build: check in XModule SCSS entry files instead of generating them 

    xmodule_assets formerly generated a series of SCSS entry
files, which imported from SCSS sources in xmodule/static/sass.
This adds an unnecessary and confusing step to the SCSS build.

Instead, we move the generated files to xmodule/sass, and TODO

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    9867bc4 View commit details
    Browse the repository at this point in the history

  6. style: add newline at end of XModule SCSS files 

    Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    7f619e5 View commit details
    Browse the repository at this point in the history

  7. build: xmodule/sass -> xmodule/static/sass 

    Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    17d3487 View commit details
    Browse the repository at this point in the history

  8. build: common/static/css/xmodule -> xmodule/static/css 

    Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    85f6f0a View commit details
    Browse the repository at this point in the history

  9. build: reference original XModule JS instead of copying 

    TODO: details

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    b727142 View commit details
    Browse the repository at this point in the history

  10. build: check in XModule webpack config; delete xmodule_assets script 

    TODO:
* commit details
* understand PIPELINE['javascript']
* look for any other code that expects modules at old generated
  locations

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    dd25c77 View commit details
    Browse the repository at this point in the history

  11. build: xmodule/js -> xmodule/static/js 

    TODO:
* commit details
* understand PIPELINE['javascript']
* look for any other code that expects modules at
  xmodule/js instead of xmodule/static/js

Part of: openedx#31624
    @kdmccormick
    kdmccormick committed May 22, 2023
    Configuration menu
    Copy the full SHA
    ea3160d View commit details
    Browse the repository at this point in the history