Conversation
| ) | ||
|
|
||
| return self.map_chunk_files_to_url(filtered_chunks) | ||
| return self.map_chunk_files_to_url(filtered_chunks, assets=assets) |
There was a problem hiding this comment.
The idea of this changes is to pass down the assets instance that was obtained on line 199 in this method, instead of always relying on re-fetching the assets inside map_chunk_files_to_url, as there could be a chance that the stats file is recompiled between those calls and the data becomes inconsistent.
It's a hard-to-reproduce situation at the end of the day and that would only affect development, if it ever shows up.
| assets = self._assets.get(self.name) | ||
| if not assets or assets.get("status") == "compile": | ||
| assets = self.load_assets() | ||
| if assets.get("status") != "compile": |
There was a problem hiding this comment.
Ensure that we can only cache assets that aren't compiling after the polling in the previous line.
| @@ -82,8 +82,10 @@ def get_files( | |||
| return result | |||
|
|
|||
| used_urls = getattr(request, '_webpack_loader_used_urls', None) | |||
There was a problem hiding this comment.
used_urls was read-only before this change, which led to some non-deterministic results depending on which templatetags where used before it (regression tests added to test the interaction between get_files and render_bundle).
| "webpack_loader/templatetags", | ||
| "webpack_loader/contrib", | ||
| "webpack_loader.templatetags", | ||
| "webpack_loader.contrib", |
There was a problem hiding this comment.
Unrelated change. Should this really be changed? Check what's the proper syntax.
There was a problem hiding this comment.
I've checked around and it seems that the most recommended approach is to delegate this to setuptools.find_packages https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#packages.
That function itself returns the package using the dot notation instead of the slashes.
| tag_dict = get_as_url_to_tag_dict('resources', extension='js', attrs='', request=request_with_nonce) | ||
| self.assertNotIn('nonce=', tag_dict['/static/django_webpack_loader_bundles/resources.js']) | ||
|
|
||
| def test_get_url_to_tag_dict_js_preload_includes_integrity_and_nonce(self): |
There was a problem hiding this comment.
Should we always include integrity and nonce? Any downsides when they are included? Any issues when they're empty or they are really always filled?
There was a problem hiding this comment.
They aren't always filled, if the corresponding setting for each of them was turned off the helper function would return an empty string to be added to the formatted output.
I've refactored that behavior to improve readability in this flow here a978459. With that we got rid of the {0},{1},{2}, etc formats that were starting to get a bit out of control in favor of having those parameters being conditionally added to a list and then remove the null-ish values.
I think this will also make it easier for a developer reading the code to check that those values might be empty.
2 participants
Closes #426
Closes #429
Closes #430
Closes #431
TemplateView.rendered_contentmultiple times per-request, as this was impacting the request used urls propertyCloses #432