django-sass-processor icon indicating copy to clipboard operation
django-sass-processor copied to clipboard

Manifest files storage

Open jrief opened this issue 2 years ago • 4 comments

This branch is based on version 1.2.1 which is able to handle the manifest static files storage properly.

  • A unit test to check if the former works has been added.
  • All unit tests now use pathlib.Path instead of os.path....

jrief avatar Jan 19 '23 13:01 jrief

This MR adds unnecessary complexity and special-casing for one storage class. We should not do that.

As in the previous I-don't-know-how-many iterations of this topic: The ManifestStaticFilesStorage works perfectly without any changes.

I am pushing an MWE right now, and waiting for @jrief to disprove that it works.

Natureshadow avatar Jan 19 '23 13:01 Natureshadow

So, here you go:

https://github.com/Natureshadow/sass_mwe

❯ git clone https://github.com/Natureshadow/sass_mwe
Cloning into 'sass_mwe'...
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 16 (delta 1), reused 16 (delta 1), pack-reused 0
Receiving objects: 100% (16/16), 7.91 KiB | 7.91 MiB/s, done.
Resolving deltas: 100% (1/1), done.
❯ cd sass_mwe
❯ poetry install
Creating virtualenv sass-mwe in /home/nik/sass_mwe/.venv
Installing dependencies from lock file

Package operations: 9 installs, 0 updates, 0 removals

  • Installing asgiref (3.6.0)
  • Installing sqlparse (0.4.3)
  • Installing django (4.1.5)
  • Installing django-appconf (1.0.5)
  • Installing rcssmin (1.1.1)
  • Installing rjsmin (1.2.1)
  • Installing django-compressor (4.3)
  • Installing django-sass-processor (1.2.2)
  • Installing libsass (0.22.0)

Installing the current project: sass-mwe (0.1.0)
❯ poetry run ./manage.py compilescss
....................................................................................................................................................................................................
Successfully compiled 1 referred SASS/SCSS files.
❯ poetry run ./manage.py collectstatic

132 static files copied to '/home/nik/sass_mwe/static', 132 post-processed.
❯ poetry run ./manage.py runserver &
[1] 50895
    ~/sass_mwe    master !1 ?2  Performing system checks...                                                           ✔    ICE858 🚄 

System check identified no issues (0 silenced).

You have 18 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): admin, auth, contenttypes, sessions.
Run 'python manage.py migrate' to apply them.
January 19, 2023 - 13:38:34
Django version 4.1.5, using settings 'sass_mwe.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.

❯ curl http://127.0.0.1:8000/ 2>/dev/null | grep link
        <link href="/static/style.b4b34d15db26.css" rel="stylesheet" type="text/css" />

In order for me to understand why this change is needed, please prove that this simple MWE yields a different result for you.

Natureshadow avatar Jan 19 '23 13:01 Natureshadow

As per the comment in https://github.com/Natureshadow/sass_mwe/pull/1, the issue occurs when recompiling CSS during runtime, instead of properly building for deployment. IMHO, rebuilding application code (that includes stylesheets!) during runtime in production is an anti-patern in itself. Ironically, the reason I am using an S3 storage is actually because I need to rebuild stylesheets during runtime in production :D (due to limitations of Materialize).

Yet, this still doesn't mean we need a new storage class as a special case for the manifest storage. The processor should simply call .post_process() on the storage after building if the storage has that method, and that behaviour should probably we controllable by a setting.

Natureshadow avatar Jan 19 '23 16:01 Natureshadow

As per the comment in https://github.com/Natureshadow/sass_mwe/pull/1, the issue occurs when recompiling CSS during runtime, instead of properly building for deployment.

I compile all my SCSS files and collect all my CSS files during the build process of the Docker container used for deployment.

The processor should simply call .post_process() on the storage after building

that's too late. The error occurs before.

jrief avatar Jan 19 '23 16:01 jrief