Skip to content

Commit b19b39d

Browse files
karolyifjsj
karolyi
authored and
fjsj
committed
Fixes #300, failsafe request checking
1 parent 524c0ca commit b19b39d

File tree

4 files changed

+185
-30
lines changed

4 files changed

+185
-30
lines changed

‎README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,16 @@ The `is_preload=True` option in the `render_bundle` template tag can be used to
278278
</html>
279279
```
280280
281+
### Skipping the generation of multiple common chunks
282+
283+
You can use the parameter `skip_common_chunks=True` to specify that you don't want an already generated chunk be generated again in the same page.
284+
285+
In order for this option to work, django-webpack-loader requires the `request` object to be in the context, to be able to keep track of the generated chunks.
286+
287+
The `request` object is passed by default via the `django.template.context_processors.request` middleware with using the Django built-in templating system, and also with using Jinja2.
288+
289+
If you don't have `request` in the context for some reason (e.g. using `Template.render` or `render_to_string` directly without passing the request), you'll get warnings on the console.
290+
281291
### Appending file extensions
282292
283293
The `suffix` option can be used to append a string at the end of the file URL. For instance, it can be used if your webpack configuration emits compressed `.gz` files.

‎tests/app/templates/home-duplicated.jinja

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="UTF-8">
5+
<title>Example</title>
6+
{{ render_bundle('app1', 'js') }}
7+
{{ render_bundle('app2', 'js') }}
8+
</head>
9+
10+
<body>
11+
<div id="react-app"></div>
12+
{{ render_bundle('app1', 'js') }}
13+
{{ render_bundle('app2', 'js') }}
14+
</body>
15+
</html>

‎tests/app/tests/test_webpack.py

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,33 @@
44
from shutil import rmtree
55
from subprocess import call
66
from threading import Thread
7+
from unittest.mock import Mock
8+
from unittest.mock import call as MockCall
9+
from unittest.mock import patch
710

811
from django.conf import settings
9-
from django.template import engines
10-
from django.template.backends.django import Template
12+
from django.template import Context, Template, engines
1113
from django.template.response import TemplateResponse
1214
from django.test.client import RequestFactory
1315
from django.test.testcases import TestCase
1416
from django.views.generic.base import TemplateView
17+
from django_jinja.backend import Jinja2
18+
from django_jinja.backend import Template as Jinja2Template
1519
from django_jinja.builtins import DEFAULT_EXTENSIONS
1620

1721
from webpack_loader.exceptions import (WebpackBundleLookupError, WebpackError,
1822
WebpackLoaderBadStatsError,
1923
WebpackLoaderTimeoutError)
24+
from webpack_loader.templatetags.webpack_loader import _WARNING_MESSAGE
2025
from webpack_loader.utils import get_loader
2126

2227
BUNDLE_PATH = os.path.join(
2328
settings.BASE_DIR, 'assets/django_webpack_loader_bundles/')
2429
DEFAULT_CONFIG = 'DEFAULT'
2530
_OUR_EXTENSION = 'webpack_loader.contrib.jinja2ext.WebpackExtension'
2631

32+
_warn_mock = Mock()
33+
2734

2835
class LoaderTestCase(TestCase):
2936
def setUp(self):
@@ -333,6 +340,142 @@ def test_request_blocking(self):
333340
elapsed = time.time() - then
334341
self.assertTrue(elapsed < wait_for)
335342

343+
@patch(
344+
target='webpack_loader.templatetags.webpack_loader.warn',
345+
new=_warn_mock)
346+
def test_emits_warning_on_no_request_in_djangoengine(self):
347+
"""
348+
Should emit warnings on having no request in context (django
349+
template).
350+
"""
351+
self.compile_bundles('webpack.config.skipCommon.js')
352+
asset_vendor = (
353+
'<script src="/static/django_webpack_loader_bundles/vendors.js" >'
354+
'</script>')
355+
asset_app1 = (
356+
'<script src="/static/django_webpack_loader_bundles/app1.js" >'
357+
'</script>')
358+
asset_app2 = (
359+
'<script src="/static/django_webpack_loader_bundles/app2.js" >'
360+
'</script>')
361+
362+
# Shouldn't call any `warn()` here
363+
dups_template = Template(template_string=(
364+
r'{% load render_bundle from webpack_loader %}'
365+
r'{% render_bundle "app1" %}'
366+
r'{% render_bundle "app2" %}')) # type: Template
367+
output = dups_template.render(context=Context())
368+
_warn_mock.assert_not_called()
369+
self.assertEqual(output.count(asset_app1), 1)
370+
self.assertEqual(output.count(asset_app2), 1)
371+
self.assertEqual(output.count(asset_vendor), 2)
372+
373+
# Should call `warn()` here
374+
nodups_template = Template(template_string=(
375+
r'{% load render_bundle from webpack_loader %}'
376+
r'{% render_bundle "app1" %}'
377+
r'{% render_bundle "app2" skip_common_chunks=True %}')
378+
) # type: Template
379+
output = nodups_template.render(context=Context())
380+
self.assertEqual(output.count(asset_app1), 1)
381+
self.assertEqual(output.count(asset_app2), 1)
382+
self.assertEqual(output.count(asset_vendor), 2)
383+
_warn_mock.assert_called_once_with(
384+
message=_WARNING_MESSAGE, category=RuntimeWarning)
385+
386+
# Should NOT call `warn()` here
387+
_warn_mock.reset_mock()
388+
nodups_template = Template(template_string=(
389+
r'{% load render_bundle from webpack_loader %}'
390+
r'{% render_bundle "app1" %}'
391+
r'{% render_bundle "app2" skip_common_chunks=True %}')
392+
) # type: Template
393+
request = self.factory.get(path='/')
394+
output = nodups_template.render(context=Context({'request': request}))
395+
used_tags = getattr(request, '_webpack_loader_used_tags', None)
396+
self.assertIsNotNone(used_tags, msg=(
397+
'_webpack_loader_used_tags should be a property of request!'))
398+
self.assertEqual(output.count(asset_app1), 1)
399+
self.assertEqual(output.count(asset_app2), 1)
400+
self.assertEqual(output.count(asset_vendor), 1)
401+
_warn_mock.assert_not_called()
402+
_warn_mock.reset_mock()
403+
404+
@patch(
405+
target='webpack_loader.templatetags.webpack_loader.warn',
406+
new=_warn_mock)
407+
def test_emits_warning_on_no_request_in_jinja2engine(self):
408+
'Should emit warnings on having no request in context (Jinja2).'
409+
self.compile_bundles('webpack.config.skipCommon.js')
410+
settings = {
411+
'TEMPLATES': [
412+
{
413+
'NAME': 'jinja2',
414+
'BACKEND': 'django_jinja.backend.Jinja2',
415+
'APP_DIRS': True,
416+
'OPTIONS': {
417+
'match_extension': '.jinja',
418+
'extensions': DEFAULT_EXTENSIONS + [_OUR_EXTENSION],
419+
}
420+
},
421+
]
422+
}
423+
asset_vendor = (
424+
'<script src="/static/django_webpack_loader_bundles/vendors.js" >'
425+
'</script>')
426+
asset_app1 = (
427+
'<script src="/static/django_webpack_loader_bundles/app1.js" >'
428+
'</script>')
429+
asset_app2 = (
430+
'<script src="/static/django_webpack_loader_bundles/app2.js" >'
431+
'</script>')
432+
warning_call = MockCall(
433+
message=_WARNING_MESSAGE, category=RuntimeWarning)
434+
435+
# Shouldn't call any `warn()` here
436+
with self.settings(**settings):
437+
jinja2_engine = engines['jinja2'] # type: Jinja2
438+
dups_template = jinja2_engine.get_template(
439+
template_name='home-duplicated.jinja') # type: Jinja2Template
440+
output = dups_template.render()
441+
_warn_mock.assert_not_called()
442+
self.assertEqual(output.count(asset_app1), 2)
443+
self.assertEqual(output.count(asset_app2), 2)
444+
self.assertEqual(output.count(asset_vendor), 4)
445+
446+
# Should call `warn()` here
447+
with self.settings(**settings):
448+
jinja2_engine = engines['jinja2'] # type: Jinja2
449+
nodups_template = jinja2_engine.get_template(
450+
template_name='home-deduplicated.jinja'
451+
) # type: Jinja2Template
452+
output = nodups_template.render()
453+
self.assertEqual(output.count(asset_app1), 2)
454+
self.assertEqual(output.count(asset_app2), 2)
455+
self.assertEqual(output.count(asset_vendor), 4)
456+
self.assertEqual(_warn_mock.call_count, 3)
457+
self.assertListEqual(
458+
_warn_mock.call_args_list,
459+
[warning_call, warning_call, warning_call])
460+
461+
# Should NOT call `warn()` here
462+
_warn_mock.reset_mock()
463+
request = self.factory.get(path='/')
464+
with self.settings(**settings):
465+
jinja2_engine = engines['jinja2'] # type: Jinja2
466+
nodups_template = jinja2_engine.get_template(
467+
template_name='home-deduplicated.jinja'
468+
) # type: Jinja2Template
469+
output = nodups_template.render(request=request)
470+
used_tags = getattr(request, '_webpack_loader_used_tags', None)
471+
self.assertIsNotNone(used_tags, msg=(
472+
'_webpack_loader_used_tags should be a property of request!'))
473+
self.assertEqual(output.count(asset_app1), 1)
474+
self.assertEqual(output.count(asset_app2), 1)
475+
self.assertEqual(output.count(asset_vendor), 1)
476+
_warn_mock.assert_not_called()
477+
_warn_mock.reset_mock()
478+
336479
def test_skip_common_chunks_djangoengine(self):
337480
"""Test case for deduplication of modules with the django engine."""
338481
self.compile_bundles('webpack.config.skipCommon.js')

‎webpack_loader/templatetags/webpack_loader.py

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,16 @@
1-
from django.conf import settings
1+
from warnings import warn
2+
23
from django.template import Library
34
from django.utils.safestring import mark_safe
45

56
from .. import utils
67

78
register = Library()
8-
_PROC_DJTEMPLATE = 'django.template.context_processors.request'
9-
_DJ_TEMPLATEPROCESSOR = 'django.template.backends.django.DjangoTemplates'
10-
_STARTUP_ERROR = (
11-
f'Please make sure that "{_PROC_DJTEMPLATE}" is added '
12-
'to your ["OPTIONS"]["context_processors"] list in your '
13-
f'settings.TEMPLATES where the BACKEND is "{_DJ_TEMPLATEPROCESSOR}". '
14-
'django-webpack-loader needs it and cannot run without it.')
15-
16-
17-
def _is_request_in_context():
18-
for item in settings.TEMPLATES:
19-
backend = item.get('BACKEND', {})
20-
if backend == _DJ_TEMPLATEPROCESSOR:
21-
processors = set(
22-
item.get('OPTIONS', {}).get('context_processors', []))
23-
if _PROC_DJTEMPLATE not in processors:
24-
raise RuntimeError(_STARTUP_ERROR)
25-
26-
27-
# Check settings at module import time
28-
_is_request_in_context()
9+
_WARNING_MESSAGE = (
10+
'You have specified skip_common_chunks=True but the passed context '
11+
'doesn\'t have a request. django_webpack_loader needs a request object to '
12+
'filter out duplicate chunks. Please see https://github.com/django-webpack'
13+
'/django-webpack-loader#skipping-the-generation-of-multiple-common-chunks')
2914

3015

3116
@register.simple_tag(takes_context=True)
@@ -35,15 +20,17 @@ def render_bundle(
3520
tags = utils.get_as_tags(
3621
bundle_name, extension=extension, config=config, suffix=suffix,
3722
attrs=attrs, is_preload=is_preload)
38-
39-
if not hasattr(context['request'], '_webpack_loader_used_tags'):
40-
context['request']._webpack_loader_used_tags = set()
41-
42-
used_tags = context['request']._webpack_loader_used_tags
23+
request = context.get('request')
24+
if request is None:
25+
if skip_common_chunks:
26+
warn(message=_WARNING_MESSAGE, category=RuntimeWarning)
27+
return mark_safe('\n'.join(tags))
28+
used_tags = getattr(context['request'], '_webpack_loader_used_tags', None)
29+
if not used_tags:
30+
used_tags = context['request']._webpack_loader_used_tags = set()
4331
if skip_common_chunks:
4432
tags = [tag for tag in tags if tag not in used_tags]
4533
used_tags.update(tags)
46-
4734
return mark_safe('\n'.join(tags))
4835

4936

0 commit comments

Comments
 (0)