From ab297206f046dc836c2ae1f633d6d81fe9c47ccb Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 13 Nov 2022 16:40:54 -0800 Subject: [PATCH 01/19] allow users to configure html head --- src/client/index.html | 8 +-- src/idom/backend/_asgi.py | 42 ----------- src/idom/backend/_common.py | 116 ++++++++++++++++++++++++++++++ src/idom/backend/_urls.py | 7 -- src/idom/backend/flask.py | 52 +++++++------- src/idom/backend/sanic.py | 90 +++++++++++------------ src/idom/backend/starlette.py | 59 ++++++++------- src/idom/backend/tornado.py | 85 +++++++++++----------- src/idom/backend/utils.py | 33 --------- src/idom/utils.py | 65 ++++++++++++++++- tests/test_backend/test_common.py | 2 +- 11 files changed, 324 insertions(+), 235 deletions(-) delete mode 100644 src/idom/backend/_asgi.py create mode 100644 src/idom/backend/_common.py delete mode 100644 src/idom/backend/_urls.py diff --git a/src/client/index.html b/src/client/index.html index 622ec0a51..87f0244f2 100644 --- a/src/client/index.html +++ b/src/client/index.html @@ -2,12 +2,8 @@ - - IDOM + + {__head__}
diff --git a/src/idom/backend/_asgi.py b/src/idom/backend/_asgi.py deleted file mode 100644 index 94eaa2b88..000000000 --- a/src/idom/backend/_asgi.py +++ /dev/null @@ -1,42 +0,0 @@ -from __future__ import annotations - -import asyncio -from typing import Any, Awaitable - -from asgiref.typing import ASGIApplication -from uvicorn.config import Config as UvicornConfig -from uvicorn.server import Server as UvicornServer - - -async def serve_development_asgi( - app: ASGIApplication | Any, - host: str, - port: int, - started: asyncio.Event | None, -) -> None: - """Run a development server for starlette""" - server = UvicornServer( - UvicornConfig( - app, - host=host, - port=port, - loop="asyncio", - reload=True, - ) - ) - - coros: list[Awaitable[Any]] = [server.serve()] - - if started: - coros.append(_check_if_started(server, started)) - - try: - await asyncio.gather(*coros) - finally: - await asyncio.wait_for(server.shutdown(), timeout=3) - - -async def _check_if_started(server: UvicornServer, started: asyncio.Event) -> None: - while not server.started: - await asyncio.sleep(0.2) - started.set() diff --git a/src/idom/backend/_common.py b/src/idom/backend/_common.py new file mode 100644 index 000000000..8c784a255 --- /dev/null +++ b/src/idom/backend/_common.py @@ -0,0 +1,116 @@ +from __future__ import annotations + +import asyncio +import os +from dataclasses import dataclass +from pathlib import Path, PurePosixPath +from typing import Any, Awaitable + +from asgiref.typing import ASGIApplication +from uvicorn.config import Config as UvicornConfig +from uvicorn.server import Server as UvicornServer + +from idom import __file__ as _idom_file_path +from idom import html +from idom.config import IDOM_WEB_MODULES_DIR +from idom.core.types import VdomDict +from idom.utils import vdom_to_html + + +PATH_PREFIX = PurePosixPath("/_idom") +MODULES_PATH = PATH_PREFIX / "modules" +ASSETS_PATH = PATH_PREFIX / "assets" +STREAM_PATH = PATH_PREFIX / "stream" + +CLIENT_BUILD_DIR = Path(_idom_file_path).parent / "_client" + + +async def serve_development_asgi( + app: ASGIApplication | Any, + host: str, + port: int, + started: asyncio.Event | None, +) -> None: + """Run a development server for starlette""" + server = UvicornServer( + UvicornConfig( + app, + host=host, + port=port, + loop="asyncio", + reload=True, + ) + ) + + coros: list[Awaitable[Any]] = [server.serve()] + + if started: + coros.append(_check_if_started(server, started)) + + try: + await asyncio.gather(*coros) + finally: + await asyncio.wait_for(server.shutdown(), timeout=3) + + +async def _check_if_started(server: UvicornServer, started: asyncio.Event) -> None: + while not server.started: + await asyncio.sleep(0.2) + started.set() + + +def safe_client_build_dir_path(path: str) -> Path: + """Prevent path traversal out of :data:`CLIENT_BUILD_DIR`""" + return traversal_safe_path( + CLIENT_BUILD_DIR, + *("index.html" if path in ("", "/") else path).split("/"), + ) + + +def safe_web_modules_dir_path(path: str) -> Path: + """Prevent path traversal out of :data:`idom.config.IDOM_WEB_MODULES_DIR`""" + return traversal_safe_path(IDOM_WEB_MODULES_DIR.current, *path.split("/")) + + +def traversal_safe_path(root: str | Path, *unsafe: str | Path) -> Path: + """Raise a ``ValueError`` if the ``unsafe`` path resolves outside the root dir.""" + root = os.path.abspath(root) + + # Resolve relative paths but not symlinks - symlinks should be ok since their + # presence and where they point is under the control of the developer. + path = os.path.abspath(os.path.join(root, *unsafe)) + + if os.path.commonprefix([root, path]) != root: + # If the common prefix is not root directory we resolved outside the root dir + raise ValueError("Unsafe path") + + return Path(path) + + +def read_client_index_html(options: CommonOptions) -> str: + return ( + (CLIENT_BUILD_DIR / "index.html") + .read_text() + .format(__head__=vdom_to_html(options.head)) + ) + + +@dataclass +class CommonOptions: + """Options for IDOM's built-in backed server implementations""" + + head: VdomDict | str = vdom_to_html( + html.head( + html.title("IDOM"), + html.link( + { + "rel": "icon", + "href": "_idom/assets/idom-logo-square-small.svg", + "type": "image/svg+xml", + } + ), + ) + ) + + url_prefix: str = "" + """The URL prefix where IDOM resources will be served from""" diff --git a/src/idom/backend/_urls.py b/src/idom/backend/_urls.py deleted file mode 100644 index c2523f3d7..000000000 --- a/src/idom/backend/_urls.py +++ /dev/null @@ -1,7 +0,0 @@ -from pathlib import PurePosixPath - - -PATH_PREFIX = PurePosixPath("/_idom") -MODULES_PATH = PATH_PREFIX / "modules" -ASSETS_PATH = PATH_PREFIX / "assets" -STREAM_PATH = PATH_PREFIX / "stream" diff --git a/src/idom/backend/flask.py b/src/idom/backend/flask.py index 7a7e18c8d..5f4f34b05 100644 --- a/src/idom/backend/flask.py +++ b/src/idom/backend/flask.py @@ -9,7 +9,7 @@ from queue import Queue as ThreadQueue from threading import Event as ThreadEvent from threading import Thread -from typing import Any, Callable, Dict, NamedTuple, NoReturn, Optional, Union, cast +from typing import Any, Callable, NamedTuple, NoReturn, Optional, cast from flask import ( Blueprint, @@ -25,6 +25,17 @@ from werkzeug.serving import BaseWSGIServer, make_server import idom +from idom.backend._common import ( + ASSETS_PATH, + CLIENT_BUILD_DIR, + MODULES_PATH, + PATH_PREFIX, + STREAM_PATH, + CommonOptions, + read_client_index_html, + safe_client_build_dir_path, + safe_web_modules_dir_path, +) from idom.backend.hooks import ConnectionContext from idom.backend.hooks import use_connection as _use_connection from idom.backend.types import Connection, Location @@ -33,13 +44,6 @@ from idom.core.types import ComponentType, RootComponentConstructor from idom.utils import Ref -from ._urls import ASSETS_PATH, MODULES_PATH, PATH_PREFIX, STREAM_PATH -from .utils import ( - CLIENT_BUILD_DIR, - safe_client_build_dir_path, - safe_web_modules_dir_path, -) - logger = logging.getLogger(__name__) @@ -134,21 +138,15 @@ def use_connection() -> Connection[_FlaskCarrier]: @dataclass -class Options: +class Options(CommonOptions): """Render server config for :class:`FlaskRenderServer`""" - cors: Union[bool, Dict[str, Any]] = False + cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) For more information see docs for ``flask_cors.CORS`` """ - serve_static_files: bool = True - """Whether or not to serve static files (i.e. web modules)""" - - url_prefix: str = "" - """The URL prefix where IDOM resources will be served from""" - def _setup_common_routes( api_blueprint: Blueprint, @@ -160,20 +158,20 @@ def _setup_common_routes( cors_params = cors_options if isinstance(cors_options, dict) else {} CORS(api_blueprint, **cors_params) - if options.serve_static_files: + @api_blueprint.route(f"/{ASSETS_PATH.name}/") + def send_assets_dir(path: str = "") -> Any: + return send_file(safe_client_build_dir_path(f"assets/{path}")) - @api_blueprint.route(f"/{ASSETS_PATH.name}/") - def send_assets_dir(path: str = "") -> Any: - return send_file(safe_client_build_dir_path(f"assets/{path}")) + @api_blueprint.route(f"/{MODULES_PATH.name}/") + def send_modules_dir(path: str = "") -> Any: + return send_file(safe_web_modules_dir_path(path)) - @api_blueprint.route(f"/{MODULES_PATH.name}/") - def send_modules_dir(path: str = "") -> Any: - return send_file(safe_web_modules_dir_path(path)) + index_html = read_client_index_html(options) - @spa_blueprint.route("/") - @spa_blueprint.route("/") - def send_client_dir(_: str = "") -> Any: - return send_file(CLIENT_BUILD_DIR / "index.html") + @spa_blueprint.route("/") + @spa_blueprint.route("/") + def send_client_dir(_: str = "") -> Any: + return index_html def _setup_single_view_dispatcher_route( diff --git a/src/idom/backend/sanic.py b/src/idom/backend/sanic.py index a18b2cc66..6b506841b 100644 --- a/src/idom/backend/sanic.py +++ b/src/idom/backend/sanic.py @@ -23,16 +23,22 @@ serve_json_patch, ) from idom.core.types import RootComponentConstructor +from idom.utils import vdom_to_html -from ._asgi import serve_development_asgi -from ._urls import ASSETS_PATH, MODULES_PATH, PATH_PREFIX, STREAM_PATH -from .hooks import ConnectionContext -from .hooks import use_connection as _use_connection -from .utils import ( +from ._common import ( + ASSETS_PATH, CLIENT_BUILD_DIR, + MODULES_PATH, + PATH_PREFIX, + STREAM_PATH, + CommonOptions, + read_client_index_html, safe_client_build_dir_path, safe_web_modules_dir_path, + serve_development_asgi, ) +from .hooks import ConnectionContext +from .hooks import use_connection as _use_connection logger = logging.getLogger(__name__) @@ -90,21 +96,15 @@ def use_connection() -> Connection[_SanicCarrier]: @dataclass -class Options: +class Options(CommonOptions): """Options for :class:`SanicRenderServer`""" - cors: Union[bool, Dict[str, Any]] = False + cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) For more information see docs for ``sanic_cors.CORS`` """ - serve_static_files: bool = True - """Whether or not to serve static files (i.e. web modules)""" - - url_prefix: str = "" - """The URL prefix where IDOM resources will be served from""" - def _setup_common_routes( api_blueprint: Blueprint, @@ -116,38 +116,38 @@ def _setup_common_routes( cors_params = cors_options if isinstance(cors_options, dict) else {} CORS(api_blueprint, **cors_params) - if options.serve_static_files: - - async def single_page_app_files( - request: request.Request, - _: str = "", - ) -> response.HTTPResponse: - return await response.file(CLIENT_BUILD_DIR / "index.html") - - spa_blueprint.add_route(single_page_app_files, "/") - spa_blueprint.add_route(single_page_app_files, "/<_:path>") - - async def asset_files( - request: request.Request, - path: str = "", - ) -> response.HTTPResponse: - path = urllib_parse.unquote(path) - return await response.file(safe_client_build_dir_path(f"assets/{path}")) - - api_blueprint.add_route(asset_files, f"/{ASSETS_PATH.name}/") - - async def web_module_files( - request: request.Request, - path: str, - _: str = "", # this is not used - ) -> response.HTTPResponse: - path = urllib_parse.unquote(path) - return await response.file( - safe_web_modules_dir_path(path), - mime_type="text/javascript", - ) - - api_blueprint.add_route(web_module_files, f"/{MODULES_PATH.name}/") + index_html = read_client_index_html(options) + + async def single_page_app_files( + request: request.Request, + _: str = "", + ) -> response.HTTPResponse: + return response.html(index_html) + + spa_blueprint.add_route(single_page_app_files, "/") + spa_blueprint.add_route(single_page_app_files, "/<_:path>") + + async def asset_files( + request: request.Request, + path: str = "", + ) -> response.HTTPResponse: + path = urllib_parse.unquote(path) + return await response.file(safe_client_build_dir_path(f"assets/{path}")) + + api_blueprint.add_route(asset_files, f"/{ASSETS_PATH.name}/") + + async def web_module_files( + request: request.Request, + path: str, + _: str = "", # this is not used + ) -> response.HTTPResponse: + path = urllib_parse.unquote(path) + return await response.file( + safe_web_modules_dir_path(path), + mime_type="text/javascript", + ) + + api_blueprint.add_route(web_module_files, f"/{MODULES_PATH.name}/") def _setup_single_view_dispatcher_route( diff --git a/src/idom/backend/starlette.py b/src/idom/backend/starlette.py index ed1bc10da..26dbf3723 100644 --- a/src/idom/backend/starlette.py +++ b/src/idom/backend/starlette.py @@ -4,12 +4,12 @@ import json import logging from dataclasses import dataclass -from typing import Any, Dict, Tuple, Union +from typing import Any, Awaitable, Callable, Tuple from starlette.applications import Starlette from starlette.middleware.cors import CORSMiddleware from starlette.requests import Request -from starlette.responses import FileResponse +from starlette.responses import HTMLResponse from starlette.staticfiles import StaticFiles from starlette.websockets import WebSocket, WebSocketDisconnect @@ -25,11 +25,17 @@ ) from idom.core.types import RootComponentConstructor -from ._asgi import serve_development_asgi -from ._urls import ASSETS_PATH, MODULES_PATH, STREAM_PATH +from ._common import ( + ASSETS_PATH, + CLIENT_BUILD_DIR, + MODULES_PATH, + STREAM_PATH, + CommonOptions, + read_client_index_html, + serve_development_asgi, +) from .hooks import ConnectionContext from .hooks import use_connection as _use_connection -from .utils import CLIENT_BUILD_DIR logger = logging.getLogger(__name__) @@ -86,21 +92,15 @@ def use_connection() -> Connection[WebSocket]: @dataclass -class Options: +class Options(CommonOptions): """Optionsuration options for :class:`StarletteRenderServer`""" - cors: Union[bool, Dict[str, Any]] = False + cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) For more information see docs for ``starlette.middleware.cors.CORSMiddleware`` """ - serve_static_files: bool = True - """Whether or not to serve static files (i.e. web modules)""" - - url_prefix: str = "" - """The URL prefix where IDOM resources will be served from""" - def _setup_common_routes(options: Options, app: Starlette) -> None: cors_options = options.cors @@ -114,22 +114,27 @@ def _setup_common_routes(options: Options, app: Starlette) -> None: # BUG: https://github.com/tiangolo/fastapi/issues/1469 url_prefix = options.url_prefix - if options.serve_static_files: - app.mount( - str(MODULES_PATH), - StaticFiles(directory=IDOM_WEB_MODULES_DIR.current, check_dir=False), - ) - app.mount( - str(ASSETS_PATH), - StaticFiles(directory=CLIENT_BUILD_DIR / "assets", check_dir=False), - ) - # register this last so it takes least priority - app.add_route(url_prefix + "/", serve_index) - app.add_route(url_prefix + "/{path:path}", serve_index) + app.mount( + str(MODULES_PATH), + StaticFiles(directory=IDOM_WEB_MODULES_DIR.current, check_dir=False), + ) + app.mount( + str(ASSETS_PATH), + StaticFiles(directory=CLIENT_BUILD_DIR / "assets", check_dir=False), + ) + # register this last so it takes least priority + index_route = index_route(options) + app.add_route(url_prefix + "/", index_route) + app.add_route(url_prefix + "/{path:path}", index_route) + + +def index_route(options: Options) -> Callable[[Request], Awaitable[HTMLResponse]]: + index_html = read_client_index_html(options) + async def serve_index(request: Request) -> HTMLResponse: + return HTMLResponse(index_html) -async def serve_index(request: Request) -> FileResponse: - return FileResponse(CLIENT_BUILD_DIR / "index.html") + return serve_index def _setup_single_view_dispatcher_route( diff --git a/src/idom/backend/tornado.py b/src/idom/backend/tornado.py index 0af4bfb0d..5fea97e59 100644 --- a/src/idom/backend/tornado.py +++ b/src/idom/backend/tornado.py @@ -22,10 +22,19 @@ from idom.core.serve import VdomJsonPatch, serve_json_patch from idom.core.types import ComponentConstructor -from ._urls import ASSETS_PATH, MODULES_PATH, STREAM_PATH +from ._common import ( + ASSETS_PATH, + CLIENT_BUILD_DIR, + MODULES_PATH, + STREAM_PATH, + CommonOptions, + read_client_index_html, +) from .hooks import ConnectionContext from .hooks import use_connection as _use_connection -from .utils import CLIENT_BUILD_DIR + + +Options = CommonOptions def configure( @@ -98,50 +107,30 @@ def use_connection() -> Connection[HTTPServerRequest]: return conn -@dataclass -class Options: - """Render server options for :class:`TornadoRenderServer` subclasses""" - - serve_static_files: bool = True - """Whether or not to serve static files (i.e. web modules)""" - - url_prefix: str = "" - """The URL prefix where IDOM resources will be served from""" - - _RouteHandlerSpecs = List[Tuple[str, Type[RequestHandler], Any]] def _setup_common_routes(options: Options) -> _RouteHandlerSpecs: - handlers: _RouteHandlerSpecs = [] - - if options.serve_static_files: - handlers.append( - ( - rf"{MODULES_PATH}/(.*)", - StaticFileHandler, - {"path": str(IDOM_WEB_MODULES_DIR.current)}, - ) - ) - - handlers.append( - ( - rf"{ASSETS_PATH}/(.*)", - StaticFileHandler, - {"path": str(CLIENT_BUILD_DIR / "assets")}, - ) - ) - - # register last to give lowest priority - handlers.append( - ( - r"/(.*)", - SpaStaticFileHandler, - {"path": str(CLIENT_BUILD_DIR)}, - ) - ) - - return handlers + return [ + ( + rf"{MODULES_PATH}/(.*)", + StaticFileHandler, + {"path": str(IDOM_WEB_MODULES_DIR.current)}, + ), + ( + rf"{ASSETS_PATH}/(.*)", + StaticFileHandler, + {"path": str(CLIENT_BUILD_DIR / "assets")}, + ), + ( + r"/(.*)", + IndexHandler, + { + "path": str(CLIENT_BUILD_DIR), + "index_html": read_client_index_html(options), + }, + ), + ] def _add_handler( @@ -171,9 +160,15 @@ def _setup_single_view_dispatcher_route( ] -class SpaStaticFileHandler(StaticFileHandler): - async def get(self, _: str, include_body: bool = True) -> None: - return await super().get(str(CLIENT_BUILD_DIR / "index.html"), include_body) +class IndexHandler(RequestHandler): + + _index_html: str + + def initialize(self, index_html: str) -> None: + self._index_html = index_html + + async def get(self, _: str) -> None: + self.finish(self._index_html) class ModelStreamHandler(WebSocketHandler): diff --git a/src/idom/backend/utils.py b/src/idom/backend/utils.py index a3c5ee51a..bca5d8903 100644 --- a/src/idom/backend/utils.py +++ b/src/idom/backend/utils.py @@ -2,22 +2,17 @@ import asyncio import logging -import os import socket from contextlib import closing from importlib import import_module -from pathlib import Path from typing import Any, Iterator -import idom -from idom.config import IDOM_WEB_MODULES_DIR from idom.types import RootComponentConstructor from .types import BackendImplementation logger = logging.getLogger(__name__) -CLIENT_BUILD_DIR = Path(idom.__file__).parent / "_client" SUPPORTED_PACKAGES = ( "starlette", @@ -56,34 +51,6 @@ def run( asyncio.run(implementation.serve_development_app(app, host, port)) -def safe_client_build_dir_path(path: str) -> Path: - """Prevent path traversal out of :data:`CLIENT_BUILD_DIR`""" - return traversal_safe_path( - CLIENT_BUILD_DIR, - *("index.html" if path in ("", "/") else path).split("/"), - ) - - -def safe_web_modules_dir_path(path: str) -> Path: - """Prevent path traversal out of :data:`idom.config.IDOM_WEB_MODULES_DIR`""" - return traversal_safe_path(IDOM_WEB_MODULES_DIR.current, *path.split("/")) - - -def traversal_safe_path(root: str | Path, *unsafe: str | Path) -> Path: - """Raise a ``ValueError`` if the ``unsafe`` path resolves outside the root dir.""" - root = os.path.abspath(root) - - # Resolve relative paths but not symlinks - symlinks should be ok since their - # presence and where they point is under the control of the developer. - path = os.path.abspath(os.path.join(root, *unsafe)) - - if os.path.commonprefix([root, path]) != root: - # If the common prefix is not root directory we resolved outside the root dir - raise ValueError("Unsafe path") - - return Path(path) - - def find_available_port( host: str, port_min: int = 8000, diff --git a/src/idom/utils.py b/src/idom/utils.py index e176da660..634139cdf 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -1,5 +1,11 @@ +from __future__ import annotations + +import re +from collections.abc import Mapping +from html import escape as html_escape from itertools import chain -from typing import Any, Callable, Generic, Iterable, List, TypeVar, Union +from typing import Any, Callable, Generic, Iterable, TypeVar, cast +from warnings import warn from lxml import etree from lxml.html import fragments_fromstring @@ -56,6 +62,61 @@ def __repr__(self) -> str: return f"{type(self).__name__}({current})" +def vdom_to_html(vdom: str | VdomDict) -> str: + """Convert a VDOM dictionary into an HTML string + + Only the following keys are translated to HTML: + + - ``tagName`` + - ``attributes`` + - ``children`` (must be strings or more VDOM dicts) + """ + if isinstance(vdom, str): + return vdom + + try: + tag = vdom["tagName"] + except TypeError as error: + raise TypeError(f"Expected a VDOM dictionary or string, not {vdom}") from error + + if "attributes" in vdom: + vdom_attributes = dict(vdom["attributes"]) + vdom_attributes["style"] = _vdom_to_html_style(vdom_attributes["style"]) + attributes = " " + " ".join( + f'{k}="{html_escape(v)}"' for k, v in vdom_attributes.items() + ) + else: + attributes = "" + + if "children" in vdom: + vdom_children: list[str] = [] + for child in vdom["children"]: + if isinstance(child, (str, dict)): + vdom_children.append(vdom_to_html(cast("str | VdomDict", child))) + else: + warn( + f"Could not convert element of type {type(child).__name__!r} to HTML", + UserWarning, + ) + children = "".join(vdom_children) + else: + children = "" + + return f"<{tag}{attributes}>{children}" + + +_CAMEL_CASE_SUB_PATTERN = re.compile(r"(? str: + if not isinstance(style, Mapping): + return style + + return ";".join( + f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k)}:{v}" for k, v in style.items() + ) + + def html_to_vdom( html: str, *transforms: _ModelTransform, strict: bool = True ) -> VdomDict: @@ -195,7 +256,7 @@ def _mutate_vdom(vdom: VdomDict) -> None: def _generate_vdom_children( node: etree._Element, transforms: Iterable[_ModelTransform] -) -> List[Union[VdomDict, str]]: +) -> list[VdomDict | str]: """Generates a list of VDOM children from an lxml node. Inserts inner text and/or tail text inbetween VDOM children, if necessary. diff --git a/tests/test_backend/test_common.py b/tests/test_backend/test_common.py index f0856b209..947ba46c3 100644 --- a/tests/test_backend/test_common.py +++ b/tests/test_backend/test_common.py @@ -5,7 +5,7 @@ import idom from idom import html from idom.backend import default as default_implementation -from idom.backend._urls import PATH_PREFIX +from idom.backend._common import PATH_PREFIX from idom.backend.types import BackendImplementation, Connection, Location from idom.backend.utils import all_implementations from idom.testing import BackendFixture, DisplayFixture, poll From a7d1d3b68cbfc77b77e39a1a95392a62961dd1c8 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 18 Nov 2022 18:10:23 -0800 Subject: [PATCH 02/19] add tests --- src/idom/__init__.py | 3 +- src/idom/utils.py | 74 +++++++++++++------ tests/test_backend/test__common.py | 16 ++++ .../{test_common.py => test_all.py} | 0 tests/test_backend/test_utils.py | 14 ---- tests/test_utils.py | 67 ++++++++++++++++- 6 files changed, 136 insertions(+), 38 deletions(-) create mode 100644 tests/test_backend/test__common.py rename tests/test_backend/{test_common.py => test_all.py} (100%) diff --git a/src/idom/__init__.py b/src/idom/__init__.py index 22b95446f..ad6a34db5 100644 --- a/src/idom/__init__.py +++ b/src/idom/__init__.py @@ -18,7 +18,7 @@ from .core.layout import Layout from .core.serve import Stop from .core.vdom import vdom -from .utils import Ref, html_to_vdom +from .utils import Ref, html_to_vdom, vdom_to_html from .widgets import hotswap @@ -53,6 +53,7 @@ "use_ref", "use_scope", "use_state", + "vdom_to_html", "vdom", "web", ] diff --git a/src/idom/utils.py b/src/idom/utils.py index 634139cdf..f28e1f9be 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -62,7 +62,7 @@ def __repr__(self) -> str: return f"{type(self).__name__}({current})" -def vdom_to_html(vdom: str | VdomDict) -> str: +def vdom_to_html(value: str | VdomDict, indent: int = 0, depth: int = 0) -> str: """Convert a VDOM dictionary into an HTML string Only the following keys are translated to HTML: @@ -71,38 +71,68 @@ def vdom_to_html(vdom: str | VdomDict) -> str: - ``attributes`` - ``children`` (must be strings or more VDOM dicts) """ - if isinstance(vdom, str): - return vdom + if indent: + close_indent = f"\n{' ' * (indent * depth)}" + open_indent = close_indent if depth else close_indent[1:] + else: + open_indent = close_indent = "" + + if isinstance(value, str): + return f"{open_indent}{value}" if depth else value try: - tag = vdom["tagName"] - except TypeError as error: - raise TypeError(f"Expected a VDOM dictionary or string, not {vdom}") from error - - if "attributes" in vdom: - vdom_attributes = dict(vdom["attributes"]) - vdom_attributes["style"] = _vdom_to_html_style(vdom_attributes["style"]) - attributes = " " + " ".join( - f'{k}="{html_escape(v)}"' for k, v in vdom_attributes.items() - ) + tag = value["tagName"] + except TypeError as error: # pragma: no cover + raise TypeError(f"Expected a VDOM dictionary or string, not {value}") from error + + if "attributes" in value: + if not tag: # pragma: no cover + warn(f"Ignored attributes from element frament", UserWarning) + else: + vdom_attributes = dict(value["attributes"]) + if "style" in vdom_attributes: + vdom_attributes["style"] = _vdom_to_html_style(vdom_attributes["style"]) + for k, v in list(vdom_attributes.items()): + if not isinstance(v, (str, int)): + del vdom_attributes[k] + warn( + f"Could not convert attribute of type {type(v).__name__} to HTML attribute - {v}", + UserWarning, + ) + attributes = ( + f""" {' '.join(f'{k}="{html_escape(v)}"' for k, v in vdom_attributes.items())}""" + if vdom_attributes + else "" + ) else: attributes = "" - if "children" in vdom: - vdom_children: list[str] = [] - for child in vdom["children"]: - if isinstance(child, (str, dict)): - vdom_children.append(vdom_to_html(cast("str | VdomDict", child))) + if "children" in value: + children_list: list[str] = [] + + child: VdomDict | str + for child in value["children"]: + if isinstance(child, (dict, str)): + children_list.append(vdom_to_html(child, indent, depth + 1)) else: warn( - f"Could not convert element of type {type(child).__name__!r} to HTML", + f"Could not convert element of type {type(child).__name__!r} to HTML - {child}", UserWarning, ) - children = "".join(vdom_children) + + children = "".join(children_list) + else: children = "" - return f"<{tag}{attributes}>{children}" + if not children: + return f"{open_indent}<{tag}{attributes} />" if tag else "" + else: + return ( + f"{open_indent}<{tag}{attributes}>{children}{close_indent}" + if tag + else children + ) _CAMEL_CASE_SUB_PATTERN = re.compile(r"(? str: return style return ";".join( - f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k)}:{v}" for k, v in style.items() + f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k).lower()}:{v}" for k, v in style.items() ) diff --git a/tests/test_backend/test__common.py b/tests/test_backend/test__common.py new file mode 100644 index 000000000..40f87968f --- /dev/null +++ b/tests/test_backend/test__common.py @@ -0,0 +1,16 @@ +import pytest + +from idom.backend._common import traversal_safe_path + + +@pytest.mark.parametrize( + "bad_path", + [ + "../escaped", + "ok/../../escaped", + "ok/ok-again/../../ok-yet-again/../../../escaped", + ], +) +def test_catch_unsafe_relative_path_traversal(tmp_path, bad_path): + with pytest.raises(ValueError, match="Unsafe path"): + traversal_safe_path(tmp_path, *bad_path.split("/")) diff --git a/tests/test_backend/test_common.py b/tests/test_backend/test_all.py similarity index 100% rename from tests/test_backend/test_common.py rename to tests/test_backend/test_all.py diff --git a/tests/test_backend/test_utils.py b/tests/test_backend/test_utils.py index b55cdd990..c3cb13613 100644 --- a/tests/test_backend/test_utils.py +++ b/tests/test_backend/test_utils.py @@ -8,7 +8,6 @@ from idom.backend import flask as flask_implementation from idom.backend.utils import find_available_port from idom.backend.utils import run as sync_run -from idom.backend.utils import traversal_safe_path from idom.sample import SampleApp as SampleApp @@ -45,16 +44,3 @@ async def test_run(page: Page, exit_stack: ExitStack): await page.goto(url) await page.wait_for_selector("#sample") - - -@pytest.mark.parametrize( - "bad_path", - [ - "../escaped", - "ok/../../escaped", - "ok/ok-again/../../ok-yet-again/../../../escaped", - ], -) -def test_catch_unsafe_relative_path_traversal(tmp_path, bad_path): - with pytest.raises(ValueError, match="Unsafe path"): - traversal_safe_path(tmp_path, *bad_path.split("/")) diff --git a/tests/test_utils.py b/tests/test_utils.py index 861fc315d..00101b42a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,7 +1,8 @@ import pytest import idom -from idom.utils import HTMLParseError, html_to_vdom +from idom import html +from idom.utils import HTMLParseError, html_to_vdom, vdom_to_html def test_basic_ref_behavior(): @@ -149,3 +150,67 @@ def test_html_to_vdom_with_no_parent_node(): } assert html_to_vdom(source) == expected + + +@pytest.mark.parametrize( + "vdom_in, html_out", + [ + ( + html.div("hello"), + "
hello
", + ), + ( + html.div(object()), # ignore non-str/vdom children + "
", + ), + ( + html.div({"someAttribute": object()}), # ignore non-str/vdom attrs + "
", + ), + ( + html.div("hello", html.a({"href": "https://example.com"}, "example")), + '
helloexample
', + ), + ( + html.button({"onClick": lambda event: None}), + "
', + ), + ], +) +def test_vdom_to_html(vdom_in, html_out): + assert vdom_to_html(vdom_in) == html_out + + +def test_vdom_to_html_with_indent(): + assert ( + vdom_to_html( + html.div("hello", html.a({"href": "https://example.com"}, "example")), + indent=2, + ) + == '
\n hello\n \n example\n \n
' + ) From 5b6721c95bf2fffad45a5e0d1b7ad9be5ba6f4cb Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 18 Nov 2022 18:45:58 -0800 Subject: [PATCH 03/19] fix minor oversights --- src/idom/backend/starlette.py | 4 ++-- src/idom/backend/tornado.py | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/idom/backend/starlette.py b/src/idom/backend/starlette.py index 26dbf3723..8f84e8ee2 100644 --- a/src/idom/backend/starlette.py +++ b/src/idom/backend/starlette.py @@ -123,12 +123,12 @@ def _setup_common_routes(options: Options, app: Starlette) -> None: StaticFiles(directory=CLIENT_BUILD_DIR / "assets", check_dir=False), ) # register this last so it takes least priority - index_route = index_route(options) + index_route = _make_index_route(options) app.add_route(url_prefix + "/", index_route) app.add_route(url_prefix + "/{path:path}", index_route) -def index_route(options: Options) -> Callable[[Request], Awaitable[HTMLResponse]]: +def _make_index_route(options: Options) -> Callable[[Request], Awaitable[HTMLResponse]]: index_html = read_client_index_html(options) async def serve_index(request: Request) -> HTMLResponse: diff --git a/src/idom/backend/tornado.py b/src/idom/backend/tornado.py index 5fea97e59..5ea95d9b7 100644 --- a/src/idom/backend/tornado.py +++ b/src/idom/backend/tornado.py @@ -125,10 +125,7 @@ def _setup_common_routes(options: Options) -> _RouteHandlerSpecs: ( r"/(.*)", IndexHandler, - { - "path": str(CLIENT_BUILD_DIR), - "index_html": read_client_index_html(options), - }, + {"index_html": read_client_index_html(options)}, ), ] From 130ae9007b74a6a056ee876ec7f63f915e539ab9 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 18 Nov 2022 18:49:04 -0800 Subject: [PATCH 04/19] fix use_debug hook --- src/idom/core/hooks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/idom/core/hooks.py b/src/idom/core/hooks.py index a1353f090..dd7d0fc4b 100644 --- a/src/idom/core/hooks.py +++ b/src/idom/core/hooks.py @@ -23,6 +23,7 @@ from typing_extensions import Protocol +from idom.config import IDOM_DEBUG_MODE from idom.utils import Ref from ._thread_local import ThreadLocal @@ -212,7 +213,7 @@ def use_debug_value( memo_func = message if callable(message) else lambda: message new = use_memo(memo_func, dependencies) - if old.current != new: + if IDOM_DEBUG_MODE.current and old.current != new: old.current = new logger.debug(f"{current_hook().component} {new}") From 7c7e4b15baafda33f652017f71c4dbd83d0bc15c Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 19 Nov 2022 11:40:55 -0800 Subject: [PATCH 05/19] fix types --- src/idom/utils.py | 5 +++-- src/idom/widgets.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index f28e1f9be..bc9c97587 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -110,10 +110,11 @@ def vdom_to_html(value: str | VdomDict, indent: int = 0, depth: int = 0) -> str: if "children" in value: children_list: list[str] = [] - child: VdomDict | str for child in value["children"]: if isinstance(child, (dict, str)): - children_list.append(vdom_to_html(child, indent, depth + 1)) + children_list.append( + vdom_to_html(cast("VdomDict | str", child), indent, depth + 1) + ) else: warn( f"Could not convert element of type {type(child).__name__!r} to HTML - {child}", diff --git a/src/idom/widgets.py b/src/idom/widgets.py index b66e89348..d71192923 100644 --- a/src/idom/widgets.py +++ b/src/idom/widgets.py @@ -187,5 +187,5 @@ def swap(constructor: Callable[[], Any] | None) -> None: def _use_callable(initial_func: _Func) -> Tuple[_Func, Callable[[_Func], None]]: - state = hooks.use_state(lambda: initial_func) - return state[0], lambda new: state[1](lambda old: new) + state, set_state = hooks.use_state(lambda: initial_func) + return state, lambda new: set_state(lambda old: new) From 722095473de88ce41b6bc9ae5261dd29e57eb23f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 19 Nov 2022 12:07:23 -0800 Subject: [PATCH 06/19] test head customization --- tests/test_backend/test_all.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_backend/test_all.py b/tests/test_backend/test_all.py index 947ba46c3..0a6afb027 100644 --- a/tests/test_backend/test_all.py +++ b/tests/test_backend/test_all.py @@ -155,3 +155,18 @@ def ShowRoute(): # we can't easily narrow this check assert hook_val.current is not None + + +@pytest.mark.parametrize("imp", all_implementations()) +async def test_customized_head(imp: BackendImplementation, page): + @idom.component + def sample(): + return html.h1("the page title is customized") + + async with BackendFixture( + implementation=imp, + options=imp.Options(head=html.title("Custom Title")), + ) as server: + async with DisplayFixture(backend=server, driver=page) as display: + await display.show(sample) + assert (await display.page.title()) == "Custom Title" From 258ae4559dc1ded727fdc056ecc01b99ccf57a43 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 19 Nov 2022 12:12:54 -0800 Subject: [PATCH 07/19] fix typing/docstring issues --- src/idom/backend/_common.py | 40 ++++++++++++++++++--------- src/idom/backend/flask.py | 3 +- src/idom/backend/sanic.py | 6 ++-- src/idom/backend/starlette.py | 2 +- src/idom/backend/tornado.py | 2 +- src/idom/utils.py | 2 +- tests/test_backend/test__common.py | 44 +++++++++++++++++++++++++++++- tests/test_backend/test_all.py | 8 ++++-- 8 files changed, 81 insertions(+), 26 deletions(-) diff --git a/src/idom/backend/_common.py b/src/idom/backend/_common.py index 8c784a255..680164ee1 100644 --- a/src/idom/backend/_common.py +++ b/src/idom/backend/_common.py @@ -4,7 +4,7 @@ import os from dataclasses import dataclass from pathlib import Path, PurePosixPath -from typing import Any, Awaitable +from typing import Any, Awaitable, Sequence from asgiref.typing import ASGIApplication from uvicorn.config import Config as UvicornConfig @@ -91,26 +91,40 @@ def read_client_index_html(options: CommonOptions) -> str: return ( (CLIENT_BUILD_DIR / "index.html") .read_text() - .format(__head__=vdom_to_html(options.head)) + .format(__head__=vdom_head_elements_to_html(options.head)) ) +def vdom_head_elements_to_html(head: Sequence[VdomDict] | VdomDict | str) -> str: + if isinstance(head, str): + return head + elif isinstance(head, dict): + if head.get("tagName") == "head": + head = {**head, "tagName": ""} + return vdom_to_html(head) + else: + return vdom_to_html(html._(head)) + + @dataclass class CommonOptions: """Options for IDOM's built-in backed server implementations""" - head: VdomDict | str = vdom_to_html( - html.head( - html.title("IDOM"), - html.link( - { - "rel": "icon", - "href": "_idom/assets/idom-logo-square-small.svg", - "type": "image/svg+xml", - } - ), - ) + head: Sequence[VdomDict] | VdomDict | str = ( + html.title("IDOM"), + html.link( + { + "rel": "icon", + "href": "_idom/assets/idom-logo-square-small.svg", + "type": "image/svg+xml", + } + ), ) + """Add elements to the ```` of the application. + + For example, this can be used to customize the title of the page, link extra + scripts, or load stylesheets. + """ url_prefix: str = "" """The URL prefix where IDOM resources will be served from""" diff --git a/src/idom/backend/flask.py b/src/idom/backend/flask.py index 5f4f34b05..95c054b83 100644 --- a/src/idom/backend/flask.py +++ b/src/idom/backend/flask.py @@ -27,7 +27,6 @@ import idom from idom.backend._common import ( ASSETS_PATH, - CLIENT_BUILD_DIR, MODULES_PATH, PATH_PREFIX, STREAM_PATH, @@ -139,7 +138,7 @@ def use_connection() -> Connection[_FlaskCarrier]: @dataclass class Options(CommonOptions): - """Render server config for :class:`FlaskRenderServer`""" + """Render server config for :func:`idom.backend.flask.configure`""" cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) diff --git a/src/idom/backend/sanic.py b/src/idom/backend/sanic.py index 6b506841b..fda9d214f 100644 --- a/src/idom/backend/sanic.py +++ b/src/idom/backend/sanic.py @@ -4,7 +4,7 @@ import json import logging from dataclasses import dataclass -from typing import Any, Dict, MutableMapping, Tuple, Union +from typing import Any, MutableMapping, Tuple from urllib import parse as urllib_parse from uuid import uuid4 @@ -23,11 +23,9 @@ serve_json_patch, ) from idom.core.types import RootComponentConstructor -from idom.utils import vdom_to_html from ._common import ( ASSETS_PATH, - CLIENT_BUILD_DIR, MODULES_PATH, PATH_PREFIX, STREAM_PATH, @@ -97,7 +95,7 @@ def use_connection() -> Connection[_SanicCarrier]: @dataclass class Options(CommonOptions): - """Options for :class:`SanicRenderServer`""" + """Render server config for :func:`idom.backend.sanic.configure`""" cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) diff --git a/src/idom/backend/starlette.py b/src/idom/backend/starlette.py index 8f84e8ee2..21d5200af 100644 --- a/src/idom/backend/starlette.py +++ b/src/idom/backend/starlette.py @@ -93,7 +93,7 @@ def use_connection() -> Connection[WebSocket]: @dataclass class Options(CommonOptions): - """Optionsuration options for :class:`StarletteRenderServer`""" + """Render server config for :func:`idom.backend.starlette.configure`""" cors: bool | dict[str, Any] = False """Enable or configure Cross Origin Resource Sharing (CORS) diff --git a/src/idom/backend/tornado.py b/src/idom/backend/tornado.py index 5ea95d9b7..6145a9f9c 100644 --- a/src/idom/backend/tornado.py +++ b/src/idom/backend/tornado.py @@ -4,7 +4,6 @@ import json from asyncio import Queue as AsyncQueue from asyncio.futures import Future -from dataclasses import dataclass from typing import Any, List, Tuple, Type, Union from urllib.parse import urljoin @@ -35,6 +34,7 @@ Options = CommonOptions +"""Render server config for :func:`idom.backend.tornado.configure`""" def configure( diff --git a/src/idom/utils.py b/src/idom/utils.py index bc9c97587..13b3579d9 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -87,7 +87,7 @@ def vdom_to_html(value: str | VdomDict, indent: int = 0, depth: int = 0) -> str: if "attributes" in value: if not tag: # pragma: no cover - warn(f"Ignored attributes from element frament", UserWarning) + warn("Ignored attributes from element frament", UserWarning) else: vdom_attributes = dict(value["attributes"]) if "style" in vdom_attributes: diff --git a/tests/test_backend/test__common.py b/tests/test_backend/test__common.py index 40f87968f..11220eb68 100644 --- a/tests/test_backend/test__common.py +++ b/tests/test_backend/test__common.py @@ -1,6 +1,7 @@ import pytest -from idom.backend._common import traversal_safe_path +from idom import html +from idom.backend._common import traversal_safe_path, vdom_head_elements_to_html @pytest.mark.parametrize( @@ -14,3 +15,44 @@ def test_catch_unsafe_relative_path_traversal(tmp_path, bad_path): with pytest.raises(ValueError, match="Unsafe path"): traversal_safe_path(tmp_path, *bad_path.split("/")) + + +@pytest.mark.parametrize( + "vdom_in, html_out", + [ + ( + "example", + "example", + ), + ( + # We do not modify strings given by user. If given as VDOM we would have + # striped this head element, but since provided as string, we leav as-is. + "", + "", + ), + ( + html.head( + html.meta({"charset": "utf-8"}), + html.title("example"), + ), + # we strip the head element + 'example', + ), + ( + html._( + html.meta({"charset": "utf-8"}), + html.title("example"), + ), + 'example', + ), + ( + [ + html.meta({"charset": "utf-8"}), + html.title("example"), + ], + 'example', + ), + ], +) +def test_vdom_head_elements_to_html(vdom_in, html_out): + assert vdom_head_elements_to_html(vdom_in) == html_out diff --git a/tests/test_backend/test_all.py b/tests/test_backend/test_all.py index 0a6afb027..98036cb16 100644 --- a/tests/test_backend/test_all.py +++ b/tests/test_backend/test_all.py @@ -159,14 +159,16 @@ def ShowRoute(): @pytest.mark.parametrize("imp", all_implementations()) async def test_customized_head(imp: BackendImplementation, page): + custom_title = f"Custom Title for {imp.__name__}" + @idom.component def sample(): - return html.h1("the page title is customized") + return html.h1(f"^ Page title is customized to: '{custom_title}'") async with BackendFixture( implementation=imp, - options=imp.Options(head=html.title("Custom Title")), + options=imp.Options(head=html.title(custom_title)), ) as server: async with DisplayFixture(backend=server, driver=page) as display: await display.show(sample) - assert (await display.page.title()) == "Custom Title" + assert (await display.page.title()) == custom_title From 7ba546c93f8e03e42c31a136b7ac211f9f093091 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 19 Nov 2022 12:53:07 -0800 Subject: [PATCH 08/19] fix docs --- docs/source/_custom_js/package-lock.json | 2 +- src/idom/backend/tornado.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/_custom_js/package-lock.json b/docs/source/_custom_js/package-lock.json index b648d4992..8632d1665 100644 --- a/docs/source/_custom_js/package-lock.json +++ b/docs/source/_custom_js/package-lock.json @@ -19,7 +19,7 @@ } }, "../../../src/client/packages/idom-client-react": { - "version": "0.40.2", + "version": "0.41.0", "integrity": "sha512-pIK5eNwFSHKXg7ClpASWFVKyZDYxz59MSFpVaX/OqJFkrJaAxBuhKGXNTMXmuyWOL5Iyvb/ErwwDRxQRzMNkfQ==", "license": "MIT", "dependencies": { diff --git a/src/idom/backend/tornado.py b/src/idom/backend/tornado.py index 6145a9f9c..a9a112ffc 100644 --- a/src/idom/backend/tornado.py +++ b/src/idom/backend/tornado.py @@ -40,7 +40,7 @@ def configure( app: Application, component: ComponentConstructor, - options: Options | None = None, + options: CommonOptions | None = None, ) -> None: """Configure the necessary IDOM routes on the given app. From def061784445638c1b68c84cbda7f59966c97d34 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 19 Nov 2022 13:01:53 -0800 Subject: [PATCH 09/19] fix type anno --- src/idom/backend/_common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/idom/backend/_common.py b/src/idom/backend/_common.py index 680164ee1..90e2dea5b 100644 --- a/src/idom/backend/_common.py +++ b/src/idom/backend/_common.py @@ -4,7 +4,7 @@ import os from dataclasses import dataclass from pathlib import Path, PurePosixPath -from typing import Any, Awaitable, Sequence +from typing import Any, Awaitable, Sequence, cast from asgiref.typing import ASGIApplication from uvicorn.config import Config as UvicornConfig @@ -100,7 +100,7 @@ def vdom_head_elements_to_html(head: Sequence[VdomDict] | VdomDict | str) -> str return head elif isinstance(head, dict): if head.get("tagName") == "head": - head = {**head, "tagName": ""} + head = cast(VdomDict, {**head, "tagName": ""}) return vdom_to_html(head) else: return vdom_to_html(html._(head)) From fe2d15894721c5e10c8ddbbd84caabcf1ccd704a Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 21 Nov 2022 17:41:12 -0800 Subject: [PATCH 10/19] remove indent + simplify implementation --- src/idom/utils.py | 121 ++++++++++++++++++++------------------------ tests/test_utils.py | 27 ++++------ 2 files changed, 67 insertions(+), 81 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index 13b3579d9..127335350 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -62,7 +62,7 @@ def __repr__(self) -> str: return f"{type(self).__name__}({current})" -def vdom_to_html(value: str | VdomDict, indent: int = 0, depth: int = 0) -> str: +def vdom_to_html(value: str | VdomDict) -> str: """Convert a VDOM dictionary into an HTML string Only the following keys are translated to HTML: @@ -71,80 +71,40 @@ def vdom_to_html(value: str | VdomDict, indent: int = 0, depth: int = 0) -> str: - ``attributes`` - ``children`` (must be strings or more VDOM dicts) """ - if indent: - close_indent = f"\n{' ' * (indent * depth)}" - open_indent = close_indent if depth else close_indent[1:] - else: - open_indent = close_indent = "" if isinstance(value, str): - return f"{open_indent}{value}" if depth else value + return value try: tag = value["tagName"] except TypeError as error: # pragma: no cover raise TypeError(f"Expected a VDOM dictionary or string, not {value}") from error - if "attributes" in value: - if not tag: # pragma: no cover - warn("Ignored attributes from element frament", UserWarning) - else: - vdom_attributes = dict(value["attributes"]) - if "style" in vdom_attributes: - vdom_attributes["style"] = _vdom_to_html_style(vdom_attributes["style"]) - for k, v in list(vdom_attributes.items()): - if not isinstance(v, (str, int)): - del vdom_attributes[k] - warn( - f"Could not convert attribute of type {type(v).__name__} to HTML attribute - {v}", - UserWarning, - ) - attributes = ( - f""" {' '.join(f'{k}="{html_escape(v)}"' for k, v in vdom_attributes.items())}""" - if vdom_attributes - else "" - ) - else: - attributes = "" - - if "children" in value: - children_list: list[str] = [] - - for child in value["children"]: - if isinstance(child, (dict, str)): - children_list.append( - vdom_to_html(cast("VdomDict | str", child), indent, depth + 1) - ) - else: - warn( - f"Could not convert element of type {type(child).__name__!r} to HTML - {child}", - UserWarning, - ) + attributes = " ".join( + _vdom_to_html_attr(k, v) for k, v in value.get("attributes", {}).items() + ) - children = "".join(children_list) + if attributes: + assert tag, "Element frament may not contain attributes" + attributes = f" {attributes}" - else: - children = "" + children = "".join( + vdom_to_html(cast("VdomDict | str", c)) + if isinstance(c, (dict, str)) + else html_escape(str(c)) + for c in value.get("children", ()) + ) - if not children: - return f"{open_indent}<{tag}{attributes} />" if tag else "" - else: - return ( - f"{open_indent}<{tag}{attributes}>{children}{close_indent}" - if tag - else children + return ( + ( + f"<{tag}{attributes}>{children}" + if children + # To be safe we mark elements without children as self-closing. + # https://html.spec.whatwg.org/multipage/syntax.html#foreign-elements + else (f"<{tag}{attributes} />" if attributes else f"<{tag}/>") ) - - -_CAMEL_CASE_SUB_PATTERN = re.compile(r"(? str: - if not isinstance(style, Mapping): - return style - - return ";".join( - f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k).lower()}:{v}" for k, v in style.items() + if tag + else children ) @@ -210,6 +170,10 @@ def html_to_vdom( return vdom +class HTMLParseError(etree.LxmlSyntaxError): # type: ignore[misc] + """Raised when an HTML document cannot be parsed using strict parsing.""" + + def _etree_to_vdom( node: etree._Element, transforms: Iterable[_ModelTransform] ) -> VdomDict: @@ -313,5 +277,32 @@ def _hypen_to_camel_case(string: str) -> str: return first.lower() + remainder.title().replace("-", "") -class HTMLParseError(etree.LxmlSyntaxError): # type: ignore[misc] - """Raised when an HTML document cannot be parsed using strict parsing.""" +# Pattern for delimitting camelCase names (e.g. camelCase to camel-case) +_CAMEL_CASE_SUB_PATTERN = re.compile(r"(? str: + if key == "style": + if isinstance(value, dict): + value = ";".join( + # We lower only to normalize - CSS is case-insensitive: + # https://www.w3.org/TR/css-fonts-3/#font-family-casing + f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k).lower()}:{v}" + for k, v in value.items() + ) + elif key.startswith("data"): + # Handle data-* attribute names + key = _CAMEL_CASE_SUB_PATTERN.sub("-", key) + else: + key = _CAMEL_TO_DASH_CASE_HTML_ATTRS.get(key, key) + + # Again, we lower the attribute name only to normalize - HTML is case-insensitive: + # http://w3c.github.io/html-reference/documents.html#case-insensitivity + return f'{key.lower()}="{html_escape(str(value))}"' diff --git a/tests/test_utils.py b/tests/test_utils.py index 00101b42a..39cc6edf7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,3 +1,5 @@ +from html import escape as html_escape + import pytest import idom @@ -152,6 +154,9 @@ def test_html_to_vdom_with_no_parent_node(): assert html_to_vdom(source) == expected +SOME_OBJECT = object() + + @pytest.mark.parametrize( "vdom_in, html_out", [ @@ -160,12 +165,12 @@ def test_html_to_vdom_with_no_parent_node(): "
hello
", ), ( - html.div(object()), # ignore non-str/vdom children - "
", + html.div(SOME_OBJECT), + f"
{html_escape(str(SOME_OBJECT))}
", ), ( - html.div({"someAttribute": object()}), # ignore non-str/vdom attrs - "
", + html.div({"someAttribute": SOME_OBJECT}), + f'
', ), ( html.div("hello", html.a({"href": "https://example.com"}, "example")), @@ -173,7 +178,7 @@ def test_html_to_vdom_with_no_parent_node(): ), ( html.button({"onClick": lambda event: None}), - "
', + '
hello
example
', ), ], ) def test_vdom_to_html(vdom_in, html_out): assert vdom_to_html(vdom_in) == html_out - - -def test_vdom_to_html_with_indent(): - assert ( - vdom_to_html( - html.div("hello", html.a({"href": "https://example.com"}, "example")), - indent=2, - ) - == '
\n hello\n \n example\n \n
' - ) From 4cad15c01f67fa11e914c4d151b0d3dd22f7de08 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 21 Nov 2022 17:48:07 -0800 Subject: [PATCH 11/19] add changelog --- docs/source/about/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index 9af5293b2..4b4cf55be 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -26,6 +26,12 @@ Unreleased **Removed** - :pull:`840` - remove ``IDOM_FEATURE_INDEX_AS_DEFAULT_KEY`` option +- :pull:`835` - ``serve_static_files`` option from backend configuration + +**Added** + +- :pull:`835` - ability to customize the ```` element of IDOM's built-in client. +- :pull:`835` - ``vdom_to_html`` utility function. v0.41.0 From 3a3848b003e535d03fc95be9e41c93f52024d97e Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 21 Nov 2022 17:51:34 -0800 Subject: [PATCH 12/19] add test case for data- attributes --- tests/test_utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 39cc6edf7..a93de7d8d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -205,6 +205,12 @@ def test_html_to_vdom_with_no_parent_node(): ), '
hello
example
', ), + ( + html.div( + {"dataSomething": 1, "dataSomethingElse": 2, "dataisnotdashed": 3} + ), + '
', + ), ], ) def test_vdom_to_html(vdom_in, html_out): From 8e948bd7a8bd6d6df70355fc09f7cd4f457575f5 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 21 Nov 2022 20:23:29 -0800 Subject: [PATCH 13/19] use lxml for to html str --- src/idom/utils.py | 79 ++++++++++++++++++++++----------------------- tests/test_utils.py | 26 ++++++++++----- 2 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index 127335350..af49e3620 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -1,14 +1,11 @@ from __future__ import annotations import re -from collections.abc import Mapping -from html import escape as html_escape from itertools import chain from typing import Any, Callable, Generic, Iterable, TypeVar, cast -from warnings import warn from lxml import etree -from lxml.html import fragments_fromstring +from lxml.html import fragments_fromstring, tostring import idom from idom.core.types import VdomDict @@ -62,7 +59,7 @@ def __repr__(self) -> str: return f"{type(self).__name__}({current})" -def vdom_to_html(value: str | VdomDict) -> str: +def vdom_to_html(value: VdomDict) -> str: """Convert a VDOM dictionary into an HTML string Only the following keys are translated to HTML: @@ -71,40 +68,12 @@ def vdom_to_html(value: str | VdomDict) -> str: - ``attributes`` - ``children`` (must be strings or more VDOM dicts) """ - - if isinstance(value, str): - return value - - try: - tag = value["tagName"] - except TypeError as error: # pragma: no cover - raise TypeError(f"Expected a VDOM dictionary or string, not {value}") from error - - attributes = " ".join( - _vdom_to_html_attr(k, v) for k, v in value.get("attributes", {}).items() - ) - - if attributes: - assert tag, "Element frament may not contain attributes" - attributes = f" {attributes}" - - children = "".join( - vdom_to_html(cast("VdomDict | str", c)) - if isinstance(c, (dict, str)) - else html_escape(str(c)) - for c in value.get("children", ()) - ) - + temp_root = etree.Element("__temp__") + _add_vdom_to_etree(temp_root, value) return ( - ( - f"<{tag}{attributes}>{children}" - if children - # To be safe we mark elements without children as self-closing. - # https://html.spec.whatwg.org/multipage/syntax.html#foreign-elements - else (f"<{tag}{attributes} />" if attributes else f"<{tag}/>") - ) - if tag - else children + cast(bytes, tostring(temp_root)).decode() + # strip out temp root <__temp__> element + [10:-11] ) @@ -221,6 +190,32 @@ def _etree_to_vdom( return vdom +def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict) -> None: + try: + tag = vdom["tagName"] + except TypeError as e: + raise TypeError(f"Expected a VdomDict, not {vdom}") from e + except KeyError as e: + raise TypeError(f"Expected a VdomDict, not {vdom}") from e + + if tag: + element = etree.SubElement(parent, tag) + element.attrib.update( + _vdom_to_html_attr(k, v) for k, v in vdom.get("attributes", {}).items() + ) + else: + element = parent + + for c in vdom.get("children", []): + if isinstance(c, dict): + _add_vdom_to_etree(element, cast(VdomDict, c)) + elif len(element): + last_child = element[-1] + last_child.tail = f"{last_child.tail or ''}{c}" + else: + element.text = f"{element.text or ''}{c}" + + def _mutate_vdom(vdom: VdomDict) -> None: """Performs any necessary mutations on the VDOM attributes to meet VDOM spec. @@ -288,7 +283,7 @@ def _hypen_to_camel_case(string: str) -> str: } -def _vdom_to_html_attr(key: str, value: Any) -> str: +def _vdom_to_html_attr(key: str, value: Any) -> tuple[str, str]: if key == "style": if isinstance(value, dict): value = ";".join( @@ -303,6 +298,10 @@ def _vdom_to_html_attr(key: str, value: Any) -> str: else: key = _CAMEL_TO_DASH_CASE_HTML_ATTRS.get(key, key) + assert not callable( + value + ), f"Could not convert callable attribute {key}={value} to HTML" + # Again, we lower the attribute name only to normalize - HTML is case-insensitive: # http://w3c.github.io/html-reference/documents.html#case-insensitivity - return f'{key.lower()}="{html_escape(str(value))}"' + return key.lower(), str(value) diff --git a/tests/test_utils.py b/tests/test_utils.py index a93de7d8d..b6810a0e8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -170,23 +170,33 @@ def test_html_to_vdom_with_no_parent_node(): ), ( html.div({"someAttribute": SOME_OBJECT}), - f'
', + f'
', ), ( - html.div("hello", html.a({"href": "https://example.com"}, "example")), - '
helloexample
', + html.div( + "hello", html.a({"href": "https://example.com"}, "example"), "world" + ), + '
helloexampleworld
', ), ( html.button({"onClick": lambda event: None}), - "", + ), + ( + html._("hello ", html._("world")), + "hello world", + ), + ( + html._(html.div("hello"), html._("world")), + "
hello
world", ), ( html.div({"style": {"backgroundColor": "blue", "marginLeft": "10px"}}), - '
', + '
', ), ( html.div({"style": "background-color:blue;margin-left:10px"}), - '
', + '
', ), ( html._( @@ -203,13 +213,13 @@ def test_html_to_vdom_with_no_parent_node(): ), html.button(), ), - '
hello
example
', + '
hello
example
', ), ( html.div( {"dataSomething": 1, "dataSomethingElse": 2, "dataisnotdashed": 3} ), - '
', + '
', ), ], ) From 6b0df3416d418e317a7c61594916ab8794586533 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 21 Nov 2022 20:37:53 -0800 Subject: [PATCH 14/19] fix tsts --- tests/test_backend/test__common.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_backend/test__common.py b/tests/test_backend/test__common.py index 11220eb68..e575625a2 100644 --- a/tests/test_backend/test__common.py +++ b/tests/test_backend/test__common.py @@ -36,21 +36,21 @@ def test_catch_unsafe_relative_path_traversal(tmp_path, bad_path): html.title("example"), ), # we strip the head element - 'example', + 'example', ), ( html._( html.meta({"charset": "utf-8"}), html.title("example"), ), - 'example', + 'example', ), ( [ html.meta({"charset": "utf-8"}), html.title("example"), ], - 'example', + 'example', ), ], ) From c0e1248fd3a5999478a2bfd63360e2f7ee053801 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 22 Nov 2022 15:46:09 -0800 Subject: [PATCH 15/19] add final test --- src/idom/utils.py | 10 +++++----- tests/test_utils.py | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index af49e3620..975f918d8 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -190,13 +190,13 @@ def _etree_to_vdom( return vdom -def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict) -> None: +def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict | dict[str, Any]) -> None: try: tag = vdom["tagName"] - except TypeError as e: - raise TypeError(f"Expected a VdomDict, not {vdom}") from e except KeyError as e: - raise TypeError(f"Expected a VdomDict, not {vdom}") from e + raise TypeError(f"Expected a VDOM dict, not {vdom}") from e + else: + vdom = cast(VdomDict, vdom) if tag: element = etree.SubElement(parent, tag) @@ -208,7 +208,7 @@ def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict) -> None: for c in vdom.get("children", []): if isinstance(c, dict): - _add_vdom_to_etree(element, cast(VdomDict, c)) + _add_vdom_to_etree(element, c) elif len(element): last_child = element[-1] last_child.tail = f"{last_child.tail or ''}{c}" diff --git a/tests/test_utils.py b/tests/test_utils.py index b6810a0e8..405cdce05 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -225,3 +225,8 @@ def test_html_to_vdom_with_no_parent_node(): ) def test_vdom_to_html(vdom_in, html_out): assert vdom_to_html(vdom_in) == html_out + + +def test_vdom_to_html_error(): + with pytest.raises(TypeError, match="Expected a VDOM dict"): + vdom_to_html({"notVdom": True}) From b4f380cf9e08c54ac18b8bffa6b9df7f460ccef4 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 22 Nov 2022 16:56:30 -0800 Subject: [PATCH 16/19] minor improvements --- src/idom/utils.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index 975f918d8..13126c2a5 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -70,11 +70,10 @@ def vdom_to_html(value: VdomDict) -> str: """ temp_root = etree.Element("__temp__") _add_vdom_to_etree(temp_root, value) - return ( - cast(bytes, tostring(temp_root)).decode() - # strip out temp root <__temp__> element - [10:-11] - ) + html = cast(bytes, tostring(temp_root)).decode() + + # strip out temp root <__temp__> element + return html[10:-11] def html_to_vdom( @@ -201,7 +200,7 @@ def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict | dict[str, Any]) if tag: element = etree.SubElement(parent, tag) element.attrib.update( - _vdom_to_html_attr(k, v) for k, v in vdom.get("attributes", {}).items() + _vdom_attr_to_html_str(k, v) for k, v in vdom.get("attributes", {}).items() ) else: element = parent @@ -283,7 +282,7 @@ def _hypen_to_camel_case(string: str) -> str: } -def _vdom_to_html_attr(key: str, value: Any) -> tuple[str, str]: +def _vdom_attr_to_html_str(key: str, value: Any) -> tuple[str, str]: if key == "style": if isinstance(value, dict): value = ";".join( @@ -292,7 +291,7 @@ def _vdom_to_html_attr(key: str, value: Any) -> tuple[str, str]: f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k).lower()}:{v}" for k, v in value.items() ) - elif key.startswith("data"): + elif key.startswith("data") or key.startswith("aria"): # Handle data-* attribute names key = _CAMEL_CASE_SUB_PATTERN.sub("-", key) else: From 8a00c651355042282bf2f88688ec913164fcdcf5 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 22 Nov 2022 17:11:34 -0800 Subject: [PATCH 17/19] add comment --- src/idom/utils.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index 13126c2a5..3a6f7692d 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -59,7 +59,7 @@ def __repr__(self) -> str: return f"{type(self).__name__}({current})" -def vdom_to_html(value: VdomDict) -> str: +def vdom_to_html(vdom: VdomDict) -> str: """Convert a VDOM dictionary into an HTML string Only the following keys are translated to HTML: @@ -67,11 +67,13 @@ def vdom_to_html(value: VdomDict) -> str: - ``tagName`` - ``attributes`` - ``children`` (must be strings or more VDOM dicts) + + Parameters: + vdom: The VdomDict element to convert to HTML """ temp_root = etree.Element("__temp__") - _add_vdom_to_etree(temp_root, value) + _add_vdom_to_etree(temp_root, vdom) html = cast(bytes, tostring(temp_root)).decode() - # strip out temp root <__temp__> element return html[10:-11] @@ -208,11 +210,27 @@ def _add_vdom_to_etree(parent: etree._Element, vdom: VdomDict | dict[str, Any]) for c in vdom.get("children", []): if isinstance(c, dict): _add_vdom_to_etree(element, c) - elif len(element): - last_child = element[-1] - last_child.tail = f"{last_child.tail or ''}{c}" else: - element.text = f"{element.text or ''}{c}" + """ + LXML handles string children by storing them under `text` and `tail` + attributes of Element objects. The `text` attribute, if present, effectively + becomes that element's first child. Then the `tail` attribute, if present, + becomes a sibling that follows that element. For example, consider the + following HTML: + +

helloworld

+ + In this code sample, "hello" is the `text` attribute of the `` element + and "world" is the `tail` attribute of that same `` element. It's for + this reason that, depending on whether the element being constructed has + non-string a child element, we need to assign a `text` vs `tail` attribute + to that element or the last non-string child respectively. + """ + if len(element): + last_child = element[-1] + last_child.tail = f"{last_child.tail or ''}{c}" + else: + element.text = f"{element.text or ''}{c}" def _mutate_vdom(vdom: VdomDict) -> None: From fe485d3c166734f0ca3c1370eedb53e6c0621d60 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 22 Nov 2022 19:50:30 -0800 Subject: [PATCH 18/19] refine camel to dash conversion --- src/idom/utils.py | 31 ++++++++++++++++--------------- tests/test_utils.py | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/idom/utils.py b/src/idom/utils.py index 3a6f7692d..fde800abb 100644 --- a/src/idom/utils.py +++ b/src/idom/utils.py @@ -289,17 +289,6 @@ def _hypen_to_camel_case(string: str) -> str: return first.lower() + remainder.title().replace("-", "") -# Pattern for delimitting camelCase names (e.g. camelCase to camel-case) -_CAMEL_CASE_SUB_PATTERN = re.compile(r"(? tuple[str, str]: if key == "style": if isinstance(value, dict): @@ -309,11 +298,15 @@ def _vdom_attr_to_html_str(key: str, value: Any) -> tuple[str, str]: f"{_CAMEL_CASE_SUB_PATTERN.sub('-', k).lower()}:{v}" for k, v in value.items() ) - elif key.startswith("data") or key.startswith("aria"): - # Handle data-* attribute names + elif ( + # camel to data-* attributes + key.startswith("data") + # camel to aria-* attributes + or key.startswith("aria") + # handle special cases + or key in _DASHED_HTML_ATTRS + ): key = _CAMEL_CASE_SUB_PATTERN.sub("-", key) - else: - key = _CAMEL_TO_DASH_CASE_HTML_ATTRS.get(key, key) assert not callable( value @@ -322,3 +315,11 @@ def _vdom_attr_to_html_str(key: str, value: Any) -> tuple[str, str]: # Again, we lower the attribute name only to normalize - HTML is case-insensitive: # http://w3c.github.io/html-reference/documents.html#case-insensitivity return key.lower(), str(value) + + +# Pattern for delimitting camelCase names (e.g. camelCase to camel-case) +_CAMEL_CASE_SUB_PATTERN = re.compile(r"(? Date: Tue, 29 Nov 2022 10:51:52 -0800 Subject: [PATCH 19/19] Update test_utils.py --- tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 0b252fc64..405cdce05 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,7 +4,7 @@ import idom from idom import html -from idom.utils import _DASHED_HTML_ATTRS, HTMLParseError, html_to_vdom, vdom_to_html +from idom.utils import HTMLParseError, html_to_vdom, vdom_to_html def test_basic_ref_behavior():