django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Upgraded Bootstrap to 3.4.1 and added CSS source maps

Open adamchainz opened this issue 1 year ago • 4 comments

Fixes #8587. 3.4.1 has a few changes including an XSS security fix. Adding the CSS source maps will allow DRF to be used with Django 4.1 and ManifestStaticFilesStorage.

adamchainz avatar Aug 08 '22 12:08 adamchainz

Python 3.9 failure is from master.

adamchainz avatar Aug 08 '22 13:08 adamchainz

@adamchainz I played a bit with the issue and I found out the failing CI is because there are extra tests only for Python 3.9 which use a built version of DRF. The files included during the build are defined in MANIFEST.in and files with .css.map are not included. You need to modify the following line and add *.css.map https://github.com/encode/django-rest-framework/blob/8b2ccccbe53f855fd9ee9a06e7b7997270e26dda/MANIFEST.in#L4

You can check the CI runs on my branch: https://github.com/KOliver94/django-rest-framework/pull/1

KOliver94 avatar Aug 08 '22 19:08 KOliver94

Thank you, I"ve added to MANIFEST.in.

adamchainz avatar Aug 09 '22 10:08 adamchainz

Ah I didn't read the logs enough, the failure on master was due to the missing source map:

 dist run-test: commands[0] | ./runtests.py --no-pkgroot --staticfiles
Post-processing 'rest_framework/css/bootstrap-theme.min.css' failed!
...
INTERNALERROR>     raise ValueError(
INTERNALERROR> ValueError: The file 'rest_framework/css/bootstrap-theme.min.css.map' could not be found with <django.contrib.staticfiles.storage.ManifestStaticFilesStorage object at 0x7fba9b07da60>.

adamchainz avatar Aug 09 '22 10:08 adamchainz

Fantastic. Thanks all. 🙏🏼

tomchristie avatar Aug 10 '22 10:08 tomchristie

How do I fix this in my project? Cant run collectstatic/

tradevize avatar Aug 19 '22 00:08 tradevize

I think this fix hasn't been released yet. You can:

  • Wait for the release
  • Downgrade to Django 4.0
  • Use the development version (not recommended)

DE0CH avatar Aug 19 '22 02:08 DE0CH

You can also add source map files, even empty ones, to your project's static folder. Use the right file names: rest_framework/static/rest_framework/css/bootstrap-theme.min.css.map and rest_framework/static/rest_framework/css/bootstrap.min.css.map

adamchainz avatar Aug 19 '22 03:08 adamchainz

from cli calling heroku config:set DISABLE_COLLECTSTATIC=1 is also an option

spongyMongy avatar Aug 19 '22 19:08 spongyMongy

from cli calling heroku config:set DISABLE_COLLECTSTATIC=1 is also an option

This is not a good idea as it will break static assets for most projects.

adamchainz avatar Aug 19 '22 20:08 adamchainz

For those of you sniffing around for a quick fix, here's a reasonably portable one-liner that will touch .map files for all of the minified CSS provided by DRF:

find "$(python -c 'import rest_framework as rf; print(rf.__path__[0])')" -name '*.min.css' -exec touch '{}.map' \;

And if you're just really itchin' for those sweet, sweet sourcemaps, you can incant the following curse:

(
  cd "$(python -c 'import rest_framework as rf; print(rf.__path__[0])')" && \
  curl -fsSL "https://github.com/encode/django-rest-framework/archive/refs/heads/master.tar.gz" \
  | tar --extract --gzip --wildcards --strip-components=2 '*.map'
)

Oh, and here's that comment you'll forget to write the moment it starts working:

# HACK: Work around a compatibility issue between Django Rest Framework and Django 4.1
#
#   * Django 4.1 introduces a change to ManifestStaticFilesStorage that replaces CSS source 
#     map references in static files [0].
#   * Django Rest Framework vendors bootstrap CSS files which reference sourcemaps, but 
#     does not include them in the installed package's static files [1].
#
# TODO: Remove this hack when a new version of Django Rest Framework is released containing
#       the fix [1]
#
# [0] https://docs.djangoproject.com/en/4.1/releases/4.1/#django-contrib-staticfiles
# [1] https://github.com/encode/django-rest-framework/pull/8591

eabruzzese avatar Aug 20 '22 06:08 eabruzzese

@adamchainz Is there a plan to make a new release with this fix?

ulgens avatar Sep 09 '22 13:09 ulgens

Yes, #8599 is where that is happening.

adamchainz avatar Sep 09 '22 14:09 adamchainz

@adamchainz Thank you so much 🌷 I couldn't find that, sorry.

ulgens avatar Sep 09 '22 14:09 ulgens

I ran into this issue as well, while working through @wsvincent 's excellent Django for APIs book. My fix was somewhat complicated because I was also working within a Poetry environment.

My fix was to downgrade to 4.0.

# Rip Django out, and add a version which will stay at the latest 4.0.whatever.
poetry remove django
poetry add "django==~4.0"  # thatnks @adamchainz
poetry update

# Check that your repo's Django version is now, in fact, 4.0.whatever.
poetry run python -m django --version

# Try this again.
poetry run python manage.py collectstatic

(For anyone reading in the future, this fix comes around Chapter 4, "Library API", right when we have to start adding in whitenoise.)

hiAndrewQuinn avatar Sep 10 '22 06:09 hiAndrewQuinn

@hiAndrewQuinn Downgrading is probably the right approach there. Does the book not reccomend using a specific Django version from the start though? I know Will only advertises the book as updated for Django 4.0 at current.

Also a tip, you don't want to use Django 4.0.0, but instead 4.0.7, which is the latest release in the 4.0 series. It has a bunch of security and bug fixes. See the 4.0.7 release notes, and the previous linked versions. If you don't use the latest release of a given series, you may encounter already-fixed bugs, which is just frustrating.

adamchainz avatar Sep 10 '22 08:09 adamchainz

He does, and you are correct. I edited my code to reflect that for any future wannabe Djangoists.

hiAndrewQuinn avatar Sep 10 '22 08:09 hiAndrewQuinn