Opened 3 years ago

Closed 3 years ago

Last modified 7 months ago

#33353 closed Bug (invalid)

Can't collect static files if don't have vendor's JavaScript source map files

Reported by: Michael Owned by: nobody
Component: contrib.staticfiles Version: 4.0
Severity: Release blocker Keywords: manifeststatic storage
Cc: Adam Johnson Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Adam Johnson)

If one is using 3rd party JavaScript library, its often a minified Javascript file, that quite likely it has a source map url in a comment at the end. However its very likely that one does not have this source map files for the vendor libraries (probably only the vendor uses them).

Django version 4 introduces a new features of adjusting the URLs of these source map urls. Unfortunately if one does not have these source maps, its generates an error and stops. This seems unncessary that one can't create a release due to an unused third party file.

During storage post processes the files, if it can't file the file in the URL, please rather print a warning, or just skip replacing that url.

I would recommend if its a sourcemap, not even printing a warning. For the CSS files it could be worth printing a warning.

Change History (18)

comment:1 by Michael, 3 years ago

Type: UncategorizedBug

comment:2 by Mariusz Felisiak, 3 years ago

Cc: Adam Johnson added
Resolution: invalid
Status: newclosed
Summary: Can't collect static files if don't have vendor's Javascript source map filesCan't collect static files if don't have vendor's JavaScript source map files

As far as I'm aware, if you use 3rd-party libraries which have JavaScript source map references then you should also have them. A minified JavaScript file shouldn't contain source map references if map is not provided by the vendor. I hope that makes sense.

comment:3 by Adam Johnson, 3 years ago

I agree with Mariusz. From Django's perspective, it's the same as having a CSS file that references a missing font. Both are missing references and potentially a mistake! Even if you left the source map line in, the browser devtools will request it and hit a 404, and show you an error.

If you don't want the source map, delete the source map reference from the vendored file.

However its very likely that one does not have this source map files for the vendor libraries (probably only the vendor uses them).

This is just a difference in approach/opinion. I use source maps everywhere and I know many developers do too.

comment:4 by Michael, 3 years ago

Resolution: invalid
Status: closednew

I updated the original issue.

Could we please at least add a settings options to allow one to disable this new behaviour that breaks quite a few packages?

For example using django-json-widget, uses some javascript files. And contry to stated above, it does not come with the sourcemaps, and one can't collectstatic.

Last edited 3 years ago by Michael (previous) (diff)

in reply to:  2 comment:5 by Michael, 3 years ago

Replying to Mariusz Felisiak:

As far as I'm aware, if you use 3rd-party libraries which have JavaScript source map references then you should also have them. A minified JavaScript file shouldn't contain source map references if map is not provided by the vendor. I hope that makes sense.

Although this makes sense, and they "should", it does not mean that they do. One has no control over source of third party packages, which do often do not include the source maps, e.g. django-json-widget. This is a breaking change, see: https://github.com/jmrivas86/django-json-widget/issues/63

Last edited 3 years ago by Michael (previous) (diff)

comment:6 by Michael, 3 years ago

Description: modified (diff)

comment:7 by Michael, 3 years ago

Description: modified (diff)

comment:8 by Michael, 3 years ago

Description: modified (diff)

comment:9 by Michael, 3 years ago

Description: modified (diff)

comment:10 by Michael, 3 years ago

Description: modified (diff)

comment:11 by Michael, 3 years ago

Description: modified (diff)

comment:12 by Michael, 3 years ago

For anyone else stuck on this problems, this is how you can get around it:

from django.contrib.staticfiles.storage import ManifestStaticFilesStorage


module_patterns = (
    ('*.js', (
        (
            r"""(^|;)\s*(?P<matched>(?P<prefix>import(\s*\*\s*as\s+\w+\s+|\s+\w+\s+|\s*{[\w,\s]*?}\s*)from\s*)['"](?P<url>.*?)['"])""",
            "%(prefix)s '%(url)s'",
        ),
    )),
)


def get_static_url(path):
    from django.contrib.staticfiles.storage import staticfiles_storage

    return staticfiles_storage.url(path)


class LcManifestStaticFilesStorage(ManifestStaticFilesStorage):
    """
    ManifestStaticFilesStorage with additional features:
    1. If you have a manifest files error, the page to render the error probably
    has the same error, and then you can't debug it. This returns the error
    string so one can see in the rendered html there was a problem
    2. Remove the sourcemap patterns, add javascript module support.
    """

    # ------ Temp until: https://code.djangoproject.com/ticket/33353#comment:11
    no_sourcemap_patterns = (
        ("*.css", (
            r"""(?P<matched>url\(['"]{0,1}\s*(?P<url>.*?)["']{0,1}\))""",
            (
                r"""(?P<matched>@import\s*["']\s*(?P<url>.*?)["'])""",
                """@import url("%(url)s")""",
            ),
        )),
    )
    patterns = no_sourcemap_patterns + module_patterns
    # ------
    # patterns = ManifestStaticFilesStorage.patterns + module_patterns

    def stored_name(self, name):
        try:
            return super().stored_name(name)
        except ValueError as e:
            return f"{name}.could_not_find_static_file_to_calc_manifest_hash, {e}"
Last edited 3 years ago by Michael (previous) (diff)

comment:13 by Adam Johnson, 3 years ago

Description: modified (diff)

Could we please at least add a settings options to allow one to disable this new behaviour that breaks quite a few packages?

The assertion "breaks quite a few packages" is not sustained by the evidence of *one* package. IMO there's still not enough evidence to revert.

The package in question isn't tested with Django 3.2 nor 4.0. Try helping there!

You're right that you can revert the behaviour for your project by defining your own static files storage with the pattern removed. I was just writing this version before your comment came in:

from django.contrib.staticfiles.storage import ManifestStaticFilesStorage


class NoSourceMapsStorage(ManifestStaticFilesStorage):
    patterns = (
        (
            "*.css",
            (
                "(?P<matched>url\\(['\"]{0,1}\\s*(?P<url>.*?)[\"']{0,1}\\))",
                (
                    "(?P<matched>@import\\s*[\"']\\s*(?P<url>.*?)[\"'])",
                    '@import url("%(url)s")',
                ),
            ),
        ),
    )

Don't forget to set STATICFILES_STORAGE to point to your custom storage class.

comment:14 by Mariusz Felisiak, 3 years ago

Resolution: invalid
Status: newclosed

I agree with Adam. django-json-widget doesn't even officially support 3+ or Python 3.8+.

comment:15 by aodin, 2 years ago

Django Rest Framework does not include its source map files despite linking to them: https://github.com/encode/django-rest-framework/blob/fd8adb32cec9c7545039f03cc3aa6cda95d4d692/rest_framework/static/rest_framework/css/bootstrap.min.css#L6

The collectstatic command is currently broken for any builds on Django 4.1 that include DRF and use ManifestStaticFilesStorage.

Developers will need to either:

  1. Downgrade to 4.0 until DRF adds its source maps (or removes the link to them)
  2. Sub-class ManifestStaticFilesStorage with their own fix, such as disabling manifest_strict or reverting the matched patterns (both of which are mentioned above)

comment:16 by Adam Johnson, 2 years ago

I reported the DRF issue there and opened a PR to add the source maps: https://github.com/encode/django-rest-framework/issues/8587

Last edited 2 years ago by Adam Johnson (previous) (diff)

comment:17 by Rafał Pitoń, 8 months ago

I would like to add other case for this being an issue:

Webpack includes following comment in builds, but it DOESN'T include the file itself:

//# sourceMappingURL=react-hooks.esm.js.map

Presence of those comments breaks ManifestStaticFilesStorage but currently no way exists to prevent webpack from generating those short of disabling sourcemaps altogether. And there's Webpack bug: https://github.com/webpack/webpack/issues/17663

Last edited 8 months ago by Rafał Pitoń (previous) (diff)

comment:18 by David, 7 months ago

I also am experiencyng problems with this manifeststorage implementation due to how webpack build my frontend staticfiles

I have a file with the following source-map reference

//# sourceMappingURL=index.mjs.map

which comes from https://www.npmjs.com/package/fast-equals which is a inhereted dependency and is quite troublesome to remove while keeping our own source-map files.

A missing source-map file should not raise an error, rather a warning (as browser does).

It would be nice to have at least an option to "do not post-process source-map files".

Note: See TracTickets for help on using tickets.
Back to Top