From 4aa0ee35d2d6f746dc7ec0b6d01abdd6bb1ddea8 Mon Sep 17 00:00:00 2001 From: Bryan Hyshka Date: Thu, 26 May 2022 13:00:51 -0600 Subject: [PATCH 1/2] Add a setting to determine skip common chunks behaviour --- .../templates/home-duplicated-forced.jinja | 15 ++ tests/app/tests/test_webpack.py | 179 ++++++++++++++++-- webpack_loader/config.py | 1 + webpack_loader/templatetags/webpack_loader.py | 4 +- webpack_loader/utils.py | 8 + 5 files changed, 186 insertions(+), 21 deletions(-) create mode 100644 tests/app/templates/home-duplicated-forced.jinja diff --git a/tests/app/templates/home-duplicated-forced.jinja b/tests/app/templates/home-duplicated-forced.jinja new file mode 100644 index 00000000..2ffb1137 --- /dev/null +++ b/tests/app/templates/home-duplicated-forced.jinja @@ -0,0 +1,15 @@ + + + + + Example + {{ render_bundle('app1', 'js') }} + {{ render_bundle('app2', 'js', skip_common_chunks=False) }} + + + +
+ {{ render_bundle('app1', 'js', skip_common_chunks=False) }} + {{ render_bundle('app2', 'js', skip_common_chunks=False) }} + + diff --git a/tests/app/tests/test_webpack.py b/tests/app/tests/test_webpack.py index e48b686a..4894d35a 100644 --- a/tests/app/tests/test_webpack.py +++ b/tests/app/tests/test_webpack.py @@ -527,15 +527,9 @@ def test_emits_warning_on_no_request_in_jinja2engine(self): _warn_mock.assert_not_called() _warn_mock.reset_mock() - def test_skip_common_chunks_djangoengine(self): - """Test case for deduplication of modules with the django engine.""" - self.compile_bundles('webpack.config.skipCommon.js') - - django_engine = engines['django'] - dups_template = django_engine.from_string(template_code=( - r'{% load render_bundle from webpack_loader %}' - r'{% render_bundle "app1" %}' - r'{% render_bundle "app2" %}')) # type: Template + def _assert_common_chunks_duplicated_djangoengine(self, template=None): + if template is None: + raise TypeError('Django template is a required argument') request = self.factory.get(path='/') asset_vendor = ( '') - rendered_template = dups_template.render( + rendered_template = template.render( context=None, request=request) used_tags = getattr(request, '_webpack_loader_used_tags', None) @@ -556,13 +550,20 @@ def test_skip_common_chunks_djangoengine(self): self.assertEqual(rendered_template.count(asset_app2), 1) self.assertEqual(rendered_template.count(asset_vendor), 2) - nodups_template = django_engine.from_string(template_code=( - r'{% load render_bundle from webpack_loader %}' - r'{% render_bundle "app1" %}' - r'{% render_bundle "app2" skip_common_chunks=True %}') - ) # type: Template + def _assert_common_chunks_not_duplicated_djangoengine(self, template=None): + if template is None: + raise TypeError('Django template is a required argument') request = self.factory.get(path='/') - rendered_template = nodups_template.render( + asset_vendor = ( + '') + asset_app1 = ( + '') + asset_app2 = ( + '') + rendered_template = template.render( context=None, request=request) used_tags = getattr(request, '_webpack_loader_used_tags', None) @@ -572,11 +573,48 @@ def test_skip_common_chunks_djangoengine(self): self.assertEqual(rendered_template.count(asset_app2), 1) self.assertEqual(rendered_template.count(asset_vendor), 1) - def test_skip_common_chunks_jinja2engine(self): - """Test case for deduplication of modules with the Jinja2 engine.""" - self.compile_bundles('webpack.config.skipCommon.js') + def _assert_common_chunks_duplicated_jinja2engine(self, view=None): + if view is None: + raise TypeError('TemplateView is a required argument') + settings = { + 'TEMPLATES': [ + { + 'BACKEND': 'django_jinja.backend.Jinja2', + 'APP_DIRS': True, + 'OPTIONS': { + 'match_extension': '.jinja', + 'extensions': DEFAULT_EXTENSIONS + [_OUR_EXTENSION], + } + }, + ] + } + asset_vendor = ( + '') + asset_app1 = ( + '') + asset_app2 = ( + '') - view = TemplateView.as_view(template_name='home-deduplicated.jinja') + with self.settings(**settings): + request = self.factory.get('/') + result = view(request) # type: TemplateResponse + content = result.rendered_content + self.assertIn(asset_vendor, content) + self.assertIn(asset_app1, content) + self.assertIn(asset_app2, content) + self.assertEqual(content.count(asset_vendor), 4) + self.assertEqual(content.count(asset_app1), 2) + self.assertEqual(content.count(asset_app2), 2) + used_tags = getattr(request, '_webpack_loader_used_tags', None) + self.assertIsNotNone(used_tags, msg=( + '_webpack_loader_used_tags should be a property of request!')) + + def _assert_common_chunks_not_duplicated_jinja2engine(self, view=None): + if view is None: + raise TypeError('TemplateView is a required argument') settings = { 'TEMPLATES': [ { @@ -612,3 +650,104 @@ def test_skip_common_chunks_jinja2engine(self): used_tags = getattr(request, '_webpack_loader_used_tags', None) self.assertIsNotNone(used_tags, msg=( '_webpack_loader_used_tags should be a property of request!')) + + def test_skip_common_chunks_djangoengine(self): + """Test case for deduplication of modules with the django engine.""" + self.compile_bundles('webpack.config.skipCommon.js') + + django_engine = engines['django'] + dups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" %}')) # type: Template + self._assert_common_chunks_duplicated_djangoengine(dups_template) + + nodups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" skip_common_chunks=True %}') + ) # type: Template + self._assert_common_chunks_not_duplicated_djangoengine(nodups_template) + + + def test_skip_common_chunks_jinja2engine(self): + """Test case for deduplication of modules with the Jinja2 engine.""" + self.compile_bundles('webpack.config.skipCommon.js') + + view = TemplateView.as_view(template_name='home-deduplicated.jinja') + self._assert_common_chunks_not_duplicated_jinja2engine(view) + + def test_skip_common_chunks_setting_djangoengine(self): + """Test case for deduplication of modules with the django engine.""" + self.compile_bundles('webpack.config.skipCommon.js') + + django_engine = engines['django'] + dups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" %}')) # type: Template + self._assert_common_chunks_duplicated_djangoengine(dups_template) + + loader = get_loader(DEFAULT_CONFIG) + with patch.dict(loader.config, {"SKIP_COMMON_CHUNKS": True}): + self._assert_common_chunks_not_duplicated_djangoengine(dups_template) + + def test_skip_common_chunks_setting_jinja2engine(self): + """Test case for deduplication of modules with the Jinja2 engine.""" + self.compile_bundles('webpack.config.skipCommon.js') + + view = TemplateView.as_view(template_name='home-duplicated.jinja') + self._assert_common_chunks_duplicated_jinja2engine(view) + + loader = get_loader(DEFAULT_CONFIG) + with patch.dict(loader.config, {"SKIP_COMMON_CHUNKS": True}): + self._assert_common_chunks_not_duplicated_jinja2engine(view) + + def test_skip_common_chunks_setting_can_be_overridden_djangoengine(self): + """Skip common chunks template tag options should take precedent over global setting.""" + self.compile_bundles('webpack.config.skipCommon.js') + + django_engine = engines['django'] + nodups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" skip_common_chunks=True %}') + ) # type: Template + self._assert_common_chunks_not_duplicated_djangoengine(nodups_template) + + loader = get_loader(DEFAULT_CONFIG) + with patch.dict(loader.config, {"SKIP_COMMON_CHUNKS": True}): + dups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" skip_common_chunks=False %}')) # type: Template + self._assert_common_chunks_duplicated_djangoengine(dups_template) + + def test_skip_common_chunks_setting_can_be_overridden_jinja2engine(self): + """Test case for deduplication of modules with the Jinja2 engine.""" + self.compile_bundles('webpack.config.skipCommon.js') + + view = TemplateView.as_view(template_name='home-deduplicated.jinja') + self._assert_common_chunks_not_duplicated_jinja2engine(view) + + loader = get_loader(DEFAULT_CONFIG) + with patch.dict(loader.config, {"SKIP_COMMON_CHUNKS": True}): + view = TemplateView.as_view(template_name='home-duplicated-forced.jinja') + self._assert_common_chunks_duplicated_jinja2engine(view) + + def test_skip_common_chunks_missing_config(self): + self.compile_bundles('webpack.config.skipCommon.js') + + loader = get_loader(DEFAULT_CONFIG) + # remove SKIP_COMMON_CHUNKS from config completely to test backward compatibility + skip_common_chunks = loader.config.pop('SKIP_COMMON_CHUNKS') + + django_engine = engines['django'] + dups_template = django_engine.from_string(template_code=( + r'{% load render_bundle from webpack_loader %}' + r'{% render_bundle "app1" %}' + r'{% render_bundle "app2" %}')) # type: Template + self._assert_common_chunks_duplicated_djangoengine(dups_template) + + # return removed key + loader.config['SKIP_COMMON_CHUNKS'] = skip_common_chunks diff --git a/webpack_loader/config.py b/webpack_loader/config.py index 8761b2f5..0e49111a 100644 --- a/webpack_loader/config.py +++ b/webpack_loader/config.py @@ -16,6 +16,7 @@ 'IGNORE': [r'.+\.hot-update.js', r'.+\.map'], 'LOADER_CLASS': 'webpack_loader.loader.WebpackLoader', 'INTEGRITY': False, + 'SKIP_COMMON_CHUNKS': False, } } diff --git a/webpack_loader/templatetags/webpack_loader.py b/webpack_loader/templatetags/webpack_loader.py index 12081fde..f8a4890a 100644 --- a/webpack_loader/templatetags/webpack_loader.py +++ b/webpack_loader/templatetags/webpack_loader.py @@ -16,7 +16,9 @@ @register.simple_tag(takes_context=True) def render_bundle( context, bundle_name, extension=None, config='DEFAULT', suffix='', - attrs='', is_preload=False, skip_common_chunks=False): + attrs='', is_preload=False, skip_common_chunks=None): + if skip_common_chunks is None: + skip_common_chunks = utils.get_skip_common_chunks(config) tags = utils.get_as_tags( bundle_name, extension=extension, config=config, suffix=suffix, attrs=attrs, is_preload=is_preload) diff --git a/webpack_loader/utils.py b/webpack_loader/utils.py index b432dc49..ca844fac 100644 --- a/webpack_loader/utils.py +++ b/webpack_loader/utils.py @@ -28,6 +28,14 @@ def get_loader(config_name): return _loaders[config_name] +def get_skip_common_chunks(config_name): + loader = get_loader(config_name) + # The global default is currently False, whenever that is changed, change + # this fallback value as well which is present to provide backwards + # compatibility. + return loader.config.get('SKIP_COMMON_CHUNKS', False) + + def _filter_by_extension(bundle, extension): '''Return only files with the given extension''' for chunk in bundle: From 7498eb6184306877226d65870d4c674f2103215c Mon Sep 17 00:00:00 2001 From: Bryan Hyshka Date: Tue, 31 May 2022 08:59:44 -0600 Subject: [PATCH 2/2] Add a setting to configure skip common chunks behaviour globally You can now specify a 'SKIP_COMMON_CHUNKS' setting globally for each Webpack configuration. This will perform the same 'skip_common_chunks' logic from the 'render_bundle' template tag without having to add a parameter to every tag. To maintain backwards compatibility we default to 'False' and allow common chunks to be rendered to the HTML document as script tags. In the future, it will be easy to change this default behaviour if desired. I've added test coverage to ensure that the global setting can be override by the existing template tag parameter and that we maintain backwards compatibility. --- README.md | 4 ++- tests/app/tests/test_webpack.py | 57 +++++++++++++++++++++------------ webpack_loader/config.py | 2 ++ 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index b15d037e..df5875fb 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,8 @@ The `STATS_FILE` parameter represents the output file produced by `webpack-bundl - `LOADER_CLASS` is the fully qualified name of a python class as a string that holds the custom webpack loader. This is where behavior can be customized as to how the stats file is loaded. Examples include loading the stats file from a database, cache, external url, etc. For convenience, `webpack_loader.loader.WebpackLoader` can be extended. The `load_assets` method is likely where custom behavior will be added. This should return the stats file as an object. +- `SKIP_COMMON_CHUNKS` is a flag which prevents already generated chunks from being included again in the same page. This should only happen if you use more than one entrypoint per Django template (multiple `render_bundle` calls). By enabling this, you can get the same default behavior of the [HtmlWebpackPlugin](https://webpack.js.org/plugins/html-webpack-plugin/). The same caveats apply as when using `skip_common_chunks` on `render_bundle`, see that section below for more details. + Here's a simple example of loading from an external url: ```py @@ -239,7 +241,7 @@ The public path is based on `webpack.config.js` [output.publicPath](https://webp Please note that this approach will use the original asset file, and not a post-processed one from the Webpack pipeline, in case that file had gone through such flow (i.e.: You've imported an image on the React side and used it there, the file used within the React components will probably have a hash string on its name, etc. This processed file will be different than the one you'll grab with `webpack_static`). ### Use `skip_common_chunks` on `render_bundle` -You can use the parameter `skip_common_chunks=True` to specify that you don't want an already generated chunk be included again in the same page. This should only happen if you use more than one entrypoint per Django template (multiple `render_bundle` calls). By using `skip_common_chunks=True`, you can get the same default behavior of the [HtmlWebpackPlugin](https://webpack.js.org/plugins/html-webpack-plugin/). +You can use the parameter `skip_common_chunks=True` or `skip_common_chunks=False` to override the global `SKIP_COMMON_CHUNKS` setting for a specific bundle. 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. diff --git a/tests/app/tests/test_webpack.py b/tests/app/tests/test_webpack.py index 4894d35a..9ae0e66d 100644 --- a/tests/app/tests/test_webpack.py +++ b/tests/app/tests/test_webpack.py @@ -527,9 +527,13 @@ def test_emits_warning_on_no_request_in_jinja2engine(self): _warn_mock.assert_not_called() _warn_mock.reset_mock() - def _assert_common_chunks_duplicated_djangoengine(self, template=None): - if template is None: - raise TypeError('Django template is a required argument') + def _assert_common_chunks_duplicated_djangoengine(self, template): + """ + Verify that any common chunks between two bundles are duplicated in + the HTML markup. + + :param template: A Django template instance + """ request = self.factory.get(path='/') asset_vendor = ( '