Merge pull request #127 from bckohan/v2.0.3

fixes #124

Author: bckohan

doc/requirements.txt

@@ -7,4 +7,4 @@ sphinxcontrib-jsmath==1.0.1; python_version >= "3.5"
 sphinxcontrib-qthelp==1.0.3; python_version >= "3.5"
 sphinxcontrib-serializinghtml==1.1.5; python_version >= "3.5"
 sphinx-js==3.2.2; python_version >= "3.5"
-django-render-static==2.0.2
+django-render-static==2.0.3

doc/source/changelog.rst

@@ -2,6 +2,11 @@
 Change Log
 ==========
 
+v2.0.3
+======
+* Fixed `Invalid URL generation for urls with default arguments. <https://github.com/bckohan/django-render-static/issues/124>`_
+
+
 v2.0.2
 ======
 * Fixed `Dependency bug, for python < 3.9 importlib_resource req should simply be >=1.3 <https://github.com/bckohan/django-render-static/issues/123>`_

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "django-render-static"
-version = "2.0.2"
+version = "2.0.3"
 description = "Use Django's template engine to render static files at deployment or package time. Includes transpilers for extending Django's url reversal and enums to JavaScript."
 authors = ["Brian Kohan <[email protected]>"]
 license = "MIT"

render_static/__init__.py

@@ -14,7 +14,7 @@
 from .transpilers.enums_to_js import EnumClassWriter
 from .transpilers.urls_to_js import ClassURLWriter, SimpleURLWriter
 
-VERSION = (2, 0, 2)
+VERSION = (2, 0, 3)
 
 __title__ = 'Django Render Static'
 __version__ = '.'.join(str(i) for i in VERSION)

render_static/tests/js_tests.py

@@ -9,6 +9,7 @@
 from datetime import date
 from enum import Enum
 from os import makedirs
+from pathlib import Path
 from time import perf_counter
 
 import pytest
@@ -215,8 +216,8 @@ def get_members(cls):
 })
 class DefinesToJavascriptTest(StructureDiff, BaseTestCase):
 
-    def tearDown(self):
-        pass
+    # def tearDown(self):
+    #     pass
 
     def test_classes_to_js(self):
         call_command('renderstatic', 'defines1.js')
@@ -560,8 +561,8 @@ def convert_idx_to_type(arr, idx, typ):
 })
 class URLSToJavascriptTest(URLJavascriptMixin, BaseTestCase):
 
-    def tearDown(self):
-        pass
+    # def tearDown(self):
+    #     pass
 
     def setUp(self):
         self.clear_placeholder_registries()
@@ -3046,3 +3047,49 @@ def test_chained_enum_values(self):
 
     #def tearDown(self):
     #    pass
+
+
+@override_settings(
+    ROOT_URLCONF='render_static.tests.urls_default_args',
+    STATIC_TEMPLATES={
+        'ENGINES': [{
+            'BACKEND': 'render_static.backends.StaticDjangoTemplates',
+            'OPTIONS': {
+                'loaders': [
+                    ('render_static.loaders.StaticLocMemLoader', {
+                        'urls.js': '{% urls_to_js %}'
+                    })
+                ],
+                'builtins': ['render_static.templatetags.render_static']
+            },
+        }]
+    }
+)
+class SitemapURLSToJavascriptTest(URLJavascriptMixin, BaseTestCase):
+
+    # def tearDown(self):
+    #     pass
+
+    def setUp(self):
+        self.clear_placeholder_registries()
+
+    def test_sitemap_url_generation(self):
+        """
+        Test class code with legacy arguments specified individually - may be deprecated in 2.0
+        """
+        self.es6_mode = True
+        self.url_js = None
+        self.class_mode = ClassURLWriter.class_name_
+        call_command('renderstatic', 'urls.js')
+        self.assertEqual(self.get_url_from_js('sitemap'), reverse('sitemap'))
+        self.assertNotIn('sitemaps', Path(GLOBAL_STATIC_DIR / 'urls.js').read_text())
+
+        self.assertEqual(self.get_url_from_js('default', kwargs={'def': 'test'}), reverse('default', kwargs={'def': 'test'}))
+
+        self.assertIn('complex_default', Path(GLOBAL_STATIC_DIR / 'urls.js').read_text())
+
+        from render_static.tests.urls_default_args import Default
+        self.assertEqual(
+            self.get_url_from_js('default', kwargs={'def': 'blarg', 'complex_default': {'blog': Default}}),
+            reverse('default', kwargs={'def': 'blarg', 'complex_default': {'blog': Default}})
+        )

render_static/tests/settings.py

@@ -55,6 +55,7 @@
     'django.contrib.sites',
     'django.contrib.messages',
     'django.contrib.staticfiles',
+    'django.contrib.sitemaps',
     'django.contrib.admin',
 )
 

render_static/tests/urls_default_args.py

@@ -0,0 +1,39 @@
+from django.contrib.sitemaps import Sitemap
+from django.contrib.sitemaps.views import sitemap
+from django.urls import include, path
+from django.utils.timezone import now
+from render_static.tests.views import TestView
+
+
+class BlogSitemap(Sitemap):
+    changefreq = "never"
+    priority = 0.5
+
+
+class Default:
+    pass
+
+
+urlpatterns = [
+    path(
+        "sitemap.xml",
+        sitemap,
+        {"sitemaps": {
+            "blog": BlogSitemap(),
+        }},
+        name="sitemap",
+    ),
+    path(
+        'complex_default/<str:def>',
+        TestView.as_view(),
+        {'complex_default': {
+            'blog': Default
+        }},
+        name='default'
+    ),
+    path(
+        'default/<str:def>',
+        TestView.as_view(),
+        name='default'
+    ),
+] 

render_static/tests/views.py

@@ -1,7 +1,14 @@
+from django.core.serializers.json import DjangoJSONEncoder
 from django.http import JsonResponse
 from django.views import View
 
 
+class DefaultStrEncoder(DjangoJSONEncoder):
+
+    def default(self, obj):
+        return str(obj)
+
+
 class TestView(View):
 
     def get(self, request, *args, **kwargs):
@@ -16,4 +23,4 @@ def get(self, request, *args, **kwargs):
             'args': list(args),
             'kwargs': {**kwargs},
             'query': query
-        })
+        }, encoder=DefaultStrEncoder)

render_static/transpilers/urls_to_js.py

@@ -6,12 +6,14 @@
 """
 
 import itertools
+import json
 import re
 from abc import abstractmethod
 from typing import Any, Dict, Generator, Iterable, List, Optional, Tuple, Union
 
 from django import VERSION as DJANGO_VERSION
 from django.conf import settings
+from django.core.serializers.json import DjangoJSONEncoder
 from django.urls import URLPattern, URLResolver, reverse
 from django.urls.exceptions import NoReverseMatch
 from django.urls.resolvers import RegexPattern, RoutePattern
@@ -386,7 +388,8 @@ def visit_pattern(  # pylint: disable=R0914, R0915, R0912
             endpoint: URLPattern,
             qname: str,
             app_name: Optional[str],
-            route: List[RoutePattern]
+            route: List[RoutePattern],
+            num_patterns: int
     ) -> Generator[Optional[str], None, None]:
         """
         Visit a pattern. Translates the pattern into a path component string
@@ -564,7 +567,7 @@ def get_params(pattern: Union[RoutePattern, RegexPattern]) -> Dict[
 
                         yield from self.visit_path(
                             path, list(kwargs.keys()),
-                            endpoint.default_args
+                            endpoint.default_args if num_patterns > 1 else None
                         )
 
                     else:
@@ -693,7 +696,7 @@ def visit_path_group(
         yield from self.enter_path_group(qname)
         for pattern in reversed(nodes):
             yield from self.visit_pattern(
-                pattern, qname, app_name, route or []
+                pattern, qname, app_name, route or [], num_patterns=len(nodes)
             )
         yield from self.exit_path_group(qname)
 
@@ -945,7 +948,9 @@ def visit_path(
             yield f'return {quote}/{self.path_join(path).lstrip("/")}{quote};'
             self.outdent()
         else:
-            opts_str = ",".join([f"'{param}'" for param in kwargs])
+            opts_str = ",".join(
+                [self.to_javascript(param) for param in kwargs]
+            )
             yield (
                 f'if (Object.keys(kwargs).length === {len(kwargs)} && '
                 f'[{opts_str}].every(value => '
@@ -1093,6 +1098,71 @@ def reverse_jdoc(self) -> Generator[Optional[str], None, None]:
          */""".split('\n'):
             yield comment_line[8:]
 
+
+    def deep_equal(self) -> Generator[Optional[str], None, None]:
+        """
+        The recursive deepEqual function.
+        :yield: The JavaScript jdoc comment lines and deepEqual function.
+        """
+        for comment_line in """
+        /**
+         * Given two values, do a deep equality comparison. If the values are
+         * objects, all keys and values are recursively compared.
+         *
+         * @param {Object} object1 - The first object to compare.
+         * @param {Object} object2 - The second object to compare.
+         */""".split('\n'):
+            yield comment_line[8:]
+        yield 'deepEqual(object1, object2) {'
+        self.indent()
+        yield 'if (!(this.isObject(object1) && this.isObject(object2))) {'
+        self.indent()
+        yield 'return object1 === object2;'
+        self.outdent()
+        yield '}'
+        yield 'const keys1 = Object.keys(object1);'
+        yield 'const keys2 = Object.keys(object2);'
+        yield 'if (keys1.length !== keys2.length) {'
+        self.indent()
+        yield 'return false;'
+        self.outdent()
+        yield '}'
+        yield 'for (let key of keys1) {'
+        self.indent()
+        yield 'const val1 = object1[key];'
+        yield 'const val2 = object2[key];'
+        yield 'const areObjects = this.isObject(val1) && this.isObject(val2);'
+        yield 'if ('
+        self.indent()
+        yield '(areObjects && !deepEqual(val1, val2)) ||'
+        yield '(!areObjects && val1 !== val2)'
+        yield ') { return false; }'
+        self.outdent()
+        yield '}'
+        self.outdent()
+        yield 'return true;'
+        self.outdent()
+        yield '}'
+
+    def is_object(self) -> Generator[Optional[str], None, None]:
+        """
+        The isObject() function.
+        :yield: The JavaScript jdoc comment lines and isObject function.
+        """
+        for comment_line in """
+        /**
+         * Given a variable, return true if it is an object.
+         *
+         * @param {Object} object - The variable to check.
+         */""".split('\n'):
+            yield comment_line[8:]
+        yield 'isObject(object) {'
+        self.indent()
+        yield 'return object != null && typeof object === "object";'
+        self.outdent()
+        yield '}'
+
+
     def init_visit(  # pylint: disable=R0915
             self
     ) -> Generator[Optional[str], None, None]:
@@ -1159,7 +1229,7 @@ class code.
                 '{ return false; }'
             )
         else:  # pragma: no cover
-            yield 'if (kwargs[key] !== val) { return false; }'
+            yield 'if (!this.deepEqual(kwargs[key], val)) { return false; }'
         yield (
             'if (!expected.includes(key)) '
             '{ delete kwargs[key]; }'
@@ -1190,6 +1260,10 @@ class code.
         self.outdent()
         yield '}'
         yield ''
+        yield from self.deep_equal()
+        yield ''
+        yield from self.is_object()
+        yield ''
         yield from self.reverse_jdoc()
         yield 'reverse(qname, options={}) {'
         self.indent()
@@ -1331,9 +1405,20 @@ def visit_path(
         :yield: The JavaScript lines of code
         """
         quote = '`'
+        visitor = self
+        class ArgEncoder(DjangoJSONEncoder):
+            """
+            An encoder that uses the configured to javascript function to
+            convert any unknown types to strings.
+            """
+
+            def default(self, o):
+                return visitor.to_javascript(o).rstrip('"').lstrip('"')
+
+        defaults_str = json.dumps(defaults, cls=ArgEncoder)
         if len(path) == 1:  # there are no substitutions
             if defaults:
-                yield f'if (this.#match(kwargs, args, [], {defaults})) ' \
+                yield f'if (this.#match(kwargs, args, [], {defaults_str})) ' \
                       f'{{ return "/{str(path[0]).lstrip("/")}"; }}'
             else:
                 yield f'if (this.#match(kwargs, args)) ' \
@@ -1351,11 +1436,13 @@ def visit_path(
                 f'{quote}; }}'
             )
         else:
-            opts_str = ",".join([f"'{param}'" for param in kwargs])
+            opts_str = ",".join(
+                [self.to_javascript(param) for param in kwargs]
+            )
             if defaults:
                 yield (
-                    f'if (this.#match(kwargs, args, [{opts_str}], {defaults}))'
-                    f' {{'
+                    f'if (this.#match(kwargs, args, [{opts_str}], '
+                    f'{defaults_str})) {{'
                     f' return {quote}/{self.path_join(path).lstrip("/")}'
                     f'{quote}; }}'
                 )