[Django] #34497: ManifestStaticFilesStorage skips import and export of javascript modules with absolute paths
Django
with absolute paths
-------------------------------------+-------------------------------------
Reporter: Hielke | Owner: nobody
Walinga |
Type: Bug | Status: new
Component: | Version: 4.2
contrib.staticfiles | Keywords:
Severity: Normal | ManifestStaticFilesStorage ES
Triage Stage: | modules
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The recently added support for finding the ES modules does not work for
absolute paths. As this is skipped.
It is not uncommon to import from the absolute path.
{{{
import function from "/appname/file.js"
}}}
The current implementation just skips this.
A change in the code can be made to find these correctly.
Current:
{{{
# Ignore absolute/protocol-relative and data-uri URLs.
if re.match(r"^[a-z]+:", url):
return matched
# Ignore absolute URLs that don't point to a static file
(dynamic
# CSS / JS?). Note that STATIC_URL cannot be empty.
if url.startswith("/") and not
url.startswith(settings.STATIC_URL):
return matched
# Strip off the fragment so a path-like fragment won't
interfere.
url_path, fragment = urldefrag(url)
# Ignore URLs without a path
if not url_path:
return matched
if url_path.startswith("/"):
# Otherwise the condition above would have returned
prematurely.
assert url_path.startswith(settings.STATIC_URL)
target_name = url_path[len(settings.STATIC_URL) :]
else:
# We're using the posixpath module to mix paths and URLs
conveniently.
source_name = name if os.sep == "/" else
name.replace(os.sep, "/")
target_name =
posixpath.join(posixpath.dirname(source_name), url_path)
# Determine the hashed name of the target file with the
storage backend.
hashed_url = self._url(
self._stored_name,
unquote(target_name),
force=True,
hashed_files=hashed_files,
)
transformed_url = "/".join(
url_path.split("/")[:-1] + hashed_url.split("/")[-1:]
)
}}}
Proposed change:
{{{
# Ignore absolute/protocol-relative and data-uri URLs.
if re.match(r"^[a-z]+:", url):
return matched
# Strip off the fragment so a path-like fragment won't
interfere.
url_path, fragment = urldefrag(url)
# Ignore URLs without a path
if not url_path:
return matched
if url_path.startswith('/'):
if url_path.startswith(settings.STATIC_URL):
target_name = url_path[len(settings.STATIC_URL):]
else:
target_name = url_path[1:]
else:
# We're using the posixpath module to mix paths and URLs
conveniently.
source_name = name if os.sep == '/' else
name.replace(os.sep, '/')
target_name =
posixpath.join(posixpath.dirname(source_name), url_path)
try:
# Determine the hashed name of the target file with the
storage backend.
hashed_url = self._url(
self._stored_name, unquote(target_name),
force=True, hashed_files=hashed_files,
)
except ValueError:
# Ignore absolute URLs that don't point to a static file
(dynamic
# CSS / JS?). Note that STATIC_URL cannot be empty.
if url.startswith("/") and not
url.startswith(settings.STATIC_URL):
return matched
else:
raise
transformed_url = "/".join(
url_path.split("/")[:-1] + hashed_url.split("/")[-1:]
)
}}}
== PR
I could make a PR if this seems good. Thank you.
--
Ticket URL: <https://code.djangoproject.com/ticket/34497>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
with absolute paths
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
ManifestStaticFilesStorage ES |
modules |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
PR welcome, of course!
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:1>
Django
with absolute paths
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
ManifestStaticFilesStorage ES |
modules |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hielke Walinga):
Created a PR
https://github.com/django/django/pull/16775
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:2>
Django
with absolute paths
-------------------------------------+-------------------------------------
| Walinga
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
ManifestStaticFilesStorage ES |
modules |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Hielke Walinga
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:3>
Django
with absolute paths
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: Hielke
| Walinga
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
ManifestStaticFilesStorage ES |
modules |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:4>
Django
with absolute paths
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: Hielke
| Walinga
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
ManifestStaticFilesStorage ES |
modules |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: blighj (added)
Comment:
I think this is working as intended for the current way
`ManifestStaticFilesStorage` works.
If you use an absolute url that is outside /static like the example given
{{{
import function from "/appname/file.js"
}}}
I think it is correct that this is not `ManifestStaticFilesStorage`'s
responsibility. It's not part of the django project's static file, if it
was it would be in a static folder and we'd expect it to be served with
/static.
importmaps blur this line, but they aren't widly used yet, and it's not
clear to me how the code should account for them at this point.
Based on that understanding I'd suggest to close this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:5>
Django
with absolute paths
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: Hielke
| Walinga
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution: invalid
ManifestStaticFilesStorage ES |
modules |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by blighj):
* status: assigned => closed
--
Ticket URL: <https://code.djangoproject.com/ticket/34497#comment:6>