Skip to content

Fix IDOM preloader trying to parse commented out HTML templates #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ Using the following categories, list your changes in this order:
- Allow `use_mutation` to have `refetch=None`, as the docs suggest is possible.
- `use_query` will now prefetch all fields to prevent `SynchronousOnlyOperation` exceptions.
- `view_to_component`, `django_css`, and `django_js` type hints will now display like normal functions.
- IDOM preloader no longer attempts to parse commented out IDOM components.
- Tests are now fully functional on Windows

## [1.2.0] - 2022-09-19

Expand Down
1 change: 1 addition & 0 deletions requirements/test-env.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
django
playwright
twisted
channels[daphne]>=4.0.0
4 changes: 3 additions & 1 deletion src/django_idom/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
_component_tag = r"(?P<tag>component)"
_component_path = r"(?P<path>(\"[^\"'\s]+\")|('[^\"'\s]+'))"
_component_kwargs = r"(?P<kwargs>(.*?|\s*?)*)"
COMMENT_REGEX = re.compile(r"(<!--)(.|\s)*?(-->)")
COMPONENT_REGEX = re.compile(
r"{%\s*"
+ _component_tag
Expand Down Expand Up @@ -112,7 +113,8 @@ def _get_components(self, templates: set[str]) -> set[str]:
for template in templates:
with contextlib.suppress(Exception):
with open(template, "r", encoding="utf-8") as template_file:
regex_iterable = COMPONENT_REGEX.finditer(template_file.read())
clean_template = COMMENT_REGEX.sub("", template_file.read())
regex_iterable = COMPONENT_REGEX.finditer(clean_template)
component_paths = [
match.group("path").replace('"', "").replace("'", "")
for match in regex_iterable
Expand Down
10 changes: 2 additions & 8 deletions tests/test_app/tests/test_components.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import asyncio
import os
import sys
from unittest import SkipTest

from channels.testing import ChannelsLiveServerTestCase
from playwright.sync_api import TimeoutError, sync_playwright
Expand All @@ -13,13 +13,7 @@ class TestIdomCapabilities(ChannelsLiveServerTestCase):
@classmethod
def setUpClass(cls):
if sys.platform == "win32":
raise SkipTest("These tests are broken on Windows.")

# FIXME: The following lines will be needed once Django channels fixes Windows tests
# See: https://github.com/django/channels/issues/1207

# import asyncio
# asyncio.set_event_loop_policy(asyncio.WindowsProactorEventLoopPolicy())
asyncio.set_event_loop_policy(asyncio.WindowsProactorEventLoopPolicy())

# FIXME: This is required otherwise the tests will throw a `SynchronousOnlyOperation`
# error when deleting the test datatabase. Potentially a Django bug.
Expand Down
44 changes: 43 additions & 1 deletion tests/test_app/tests/test_regex.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.test import TestCase

from django_idom.utils import COMPONENT_REGEX
from django_idom.utils import COMMENT_REGEX, COMPONENT_REGEX


class RegexTests(TestCase):
Expand Down Expand Up @@ -41,3 +41,45 @@ def test_component_regex(self):
r'{{ component "" }}',
}:
self.assertNotRegex(fake_component, COMPONENT_REGEX)

def test_comment_regex(self):
for comment in {
r"<!-- comment -->",
r"""<!-- comment
-->""",
r"""<!--
comment -->""",
r"""<!--
comment
-->""",
r"""<!--
a comment
another comments
drink some cement
-->""", # noqa: W291
}:
self.assertRegex(comment, COMMENT_REGEX)

for fake_comment in {
r"<!-- a comment ",
r"another comment -->",
r"<! - - comment - - >",
r'{% component "my.component" %}',
}:
self.assertNotRegex(fake_comment, COMMENT_REGEX)

for embedded_comment in {
r'{% component "my.component" %} <!-- comment -->',
r'<!-- comment --> {% component "my.component" %}',
r'<!-- comment --> {% component "my.component" %} <!-- comment -->',
r"""<!-- comment
--> {% component "my.component" %}
<!-- comment -->
<!--
comment -->""",
}: # noqa: W291
text = COMMENT_REGEX.sub("", embedded_comment)
if text.strip() != '{% component "my.component" %}':
raise self.failureException(
f"Regex pattern {COMMENT_REGEX.pattern} failed to remove comment from {embedded_comment}"
)