From 1e6ccd2ab45dd6b56d36c23efd2a6c995f8c6e4b Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Thu, 27 Jun 2024 20:29:48 +0200 Subject: [PATCH 01/14] add mypy config to pyproject --- pyproject.toml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 51e2e06..34e5019 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,3 +33,16 @@ src = ["mjml", "tests"] lines-after-imports = 2 known-first-party = ["mjml"] combine-as-imports = true + + +[tool.mypy] +python_version = "3.8" +# strict = true +warn_unused_configs = true +warn_unused_ignores = true +check_untyped_defs = true +warn_redundant_casts = true +warn_return_any = true +warn_no_return = true +warn_unreachable = true +warn_incomplete_stub = true From cc58bb38166f11494bc1084c836230eaec75a574 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Thu, 27 Jun 2024 20:34:44 +0200 Subject: [PATCH 02/14] typing: fully typed merge_dicts --- mjml/lib/dict_merger.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/mjml/lib/dict_merger.py b/mjml/lib/dict_merger.py index 1f6c95e..35ed3ca 100644 --- a/mjml/lib/dict_merger.py +++ b/mjml/lib/dict_merger.py @@ -1,25 +1,26 @@ # -*- coding: UTF-8 -*- # Copyright 2015 Felix Schwarz # The source code in this file is licensed under the MIT license. +import typing as t -__all__ = ['merge_dicts'] +__all__ = ["merge_dicts"] -def merge_dicts(*sources): +def merge_dicts(*sources: t.Dict[str, t.Any]) -> t.Dict[str, t.Any]: # initial code from # Robin Bryce, Tue, 19 Dec 2006 # PSF license # http://code.activestate.com/recipes/499335-recursively-update-a-dictionary-without-hitting-py/ - result = {} + result: t.Dict[str, t.Any] = {} for source in sources: stack = [(source, result)] while stack: current_src, current_dst = stack.pop() for key in (current_src or ()): - src_item_is_dict = isinstance(current_src.get(key), dict) - dst_item_is_dict = isinstance(current_dst.get(key), dict) - if src_item_is_dict and dst_item_is_dict: - stack.append((current_src[key], current_dst[key])) + src_item = current_src.get(key) + dst_item = current_dst.get(key) + if isinstance(src_item, dict) and isinstance(dst_item, dict): + stack.append((src_item, dst_item)) else: - current_dst[key] = current_src[key] + current_dst[key] = src_item return result From 719ebdd283c898c66981522e61d22b0e6a59c63d Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 21:49:56 +0200 Subject: [PATCH 03/14] remove dotmap usage (at least temporarily) --- mjml/core/api.py | 39 ++++++++------ mjml/elements/_base.py | 12 ++--- mjml/elements/head/_head_base.py | 2 +- mjml/elements/head/mj_attributes.py | 2 +- mjml/elements/head/mj_html_attributes.py | 2 +- mjml/elements/mj_button.py | 2 +- mjml/elements/mj_section.py | 28 ++++++---- mjml/mjml2html.py | 66 ++++++++++++++---------- 8 files changed, 88 insertions(+), 65 deletions(-) diff --git a/mjml/core/api.py b/mjml/core/api.py index 41182f9..4a0ce7c 100644 --- a/mjml/core/api.py +++ b/mjml/core/api.py @@ -1,5 +1,4 @@ - -from dotmap import DotMap +import typing as t from ..lib import merge_dicts from .registry import components @@ -7,7 +6,8 @@ __all__ = ['initComponent', 'Component'] -def initComponent(name, **initialDatas): +def initComponent(name: t.Optional[str], + **initialDatas: t.Any) -> t.Optional["Component"]: if name is None: return None component_cls = components[name] @@ -24,15 +24,22 @@ def initComponent(name, **initialDatas): class Component: + component_name: t.ClassVar[str] + # LATER: not sure upstream also passes tagName, makes code easier for us - def __init__(self, *, attributes=None, children=(), content='', context=None, - props=None, globalAttributes=None, headStyle=None, tagName=None): + def __init__(self, *, attributes=None, children=(), content: str='', + context: t.Optional[t.Dict[str, t.Any]]=None, + props: t.Optional[t.Dict[str, t.Any]]=None, + globalAttributes: t.Optional[t.Dict[str, t.Any]]=None, + headStyle: t.Optional[t.Any]=None, + tagName: t.Optional[str]=None) -> None: self.children = list(children) self.content = content - self.context = context + # TODO typing: verify that this is the intent + self.context = context or dict() self.tagName = tagName - self.props = DotMap(merge_dicts(props, {'children': children, 'content': content})) + self.props = merge_dicts(props or {}, {'children': children, 'content': content}) # upstream also checks "self.allowed_attrs" self.attrs = merge_dicts( @@ -46,26 +53,26 @@ def __init__(self, *, attributes=None, children=(), content='', context=None, self.headStyle = headStyle @classmethod - def getTagName(cls): + def getTagName(cls) -> str: cls_name = cls.__name__ return cls_name @classmethod - def isRawElement(cls): + def isRawElement(cls) -> bool: cls_value = getattr(cls, 'rawElement', None) return bool(cls_value) # js: static defaultAttributes @classmethod - def default_attrs(cls): + def default_attrs(cls) -> t.Dict[str, t.Any]: return {} # js: static allowedAttributes @classmethod - def allowed_attrs(cls): - return () + def allowed_attrs(cls) -> t.Dict[str, str]: + return {} - def getContent(self): + def getContent(self) -> str: # Actually "self.content" should not be None but sometimes it is # (probably due to bugs in this Python port). This special guard # clause is the final fix to render the "welcome-email.mjml" from @@ -74,11 +81,11 @@ def getContent(self): return '' return self.content.strip() - def getChildContext(self): + def getChildContext(self) -> t.Dict[str, t.Any]: return self.context # js: getAttribute(name) - def get_attr(self, name, *, missing_ok=False): + def get_attr(self, name: str, *, missing_ok: bool=False) -> t.Optional[t.Any]: is_allowed_attr = name in self.allowed_attrs() is_default_attr = name in self.default_attrs() if not missing_ok and (not is_allowed_attr) and (not is_default_attr): @@ -86,5 +93,5 @@ def get_attr(self, name, *, missing_ok=False): return self.attrs.get(name) getAttribute = get_attr - def render(self): + def render(self) -> str: return '' diff --git a/mjml/elements/_base.py b/mjml/elements/_base.py index c00aac7..190acfd 100644 --- a/mjml/elements/_base.py +++ b/mjml/elements/_base.py @@ -1,6 +1,4 @@ - - -from dotmap import DotMap +import typing as t from ..core import Component, initComponent from ..core.registry import components @@ -34,19 +32,19 @@ def getShorthandBorderValue(self, direction): border = self.getAttribute('border') return borderParser(borderDirection or border or '0') - def getBoxWidths(self): + def getBoxWidths(self) -> t.Dict[str, t.Any]: containerWidth = self.context['containerWidth'] parsedWidth = strip_unit(containerWidth) get_padding = lambda d: self.getShorthandAttrValue('padding', d) paddings = get_padding('right') + get_padding('left') borders = self.getShorthandBorderValue('right') + self.getShorthandBorderValue('left') - return DotMap({ + return { 'totalWidth': parsedWidth, 'borders' : borders, 'paddings' : paddings, 'box' : parsedWidth - paddings - borders, - }) + } # js: htmlAttributes(attributes) def html_attrs(self, **attrs): @@ -100,7 +98,7 @@ def renderChildren(self, childrens=None, props=None, renderer=None, renderer = lambda component: component.render() if not attributes: attributes = {} - childrens = childrens or self.props.children + childrens = childrens or self.props.get("children") if rawXML: # return childrens.map(child => jsonToXML(child)).join('\n') diff --git a/mjml/elements/head/_head_base.py b/mjml/elements/head/_head_base.py index 7eeeed9..db6f95d 100644 --- a/mjml/elements/head/_head_base.py +++ b/mjml/elements/head/_head_base.py @@ -25,5 +25,5 @@ def handle_children(children): return component.render() return None - childrens = self.props.children + childrens = self.props.get("children") return tuple(map(handle_children, childrens)) diff --git a/mjml/elements/head/mj_attributes.py b/mjml/elements/head/mj_attributes.py index 966e1fb..bed3d47 100644 --- a/mjml/elements/head/mj_attributes.py +++ b/mjml/elements/head/mj_attributes.py @@ -11,7 +11,7 @@ class MjAttributes(HeadComponent): def handler(self): add = self.context['add'] - _children = self.props.children + _children = self.props.get("children") for child in _children: tagName = child['tagName'] diff --git a/mjml/elements/head/mj_html_attributes.py b/mjml/elements/head/mj_html_attributes.py index 7bb6169..93d953e 100644 --- a/mjml/elements/head/mj_html_attributes.py +++ b/mjml/elements/head/mj_html_attributes.py @@ -9,7 +9,7 @@ class MjHtmlAttributes(HeadComponent): def handler(self): add = self.context['add'] - _children = self.props.children + _children = self.props.get("children") for child in _children: tagName = child['tagName'] diff --git a/mjml/elements/mj_button.py b/mjml/elements/mj_button.py index e1735d7..d289145 100644 --- a/mjml/elements/mj_button.py +++ b/mjml/elements/mj_button.py @@ -124,7 +124,7 @@ def calculateAWidth(self, width): def _inner_padding(d): return self.getShorthandAttrValue('inner-padding', d, attr_with_direction=False) innerPaddings = _inner_padding('left') + _inner_padding('right') - borders = self.getBoxWidths().borders + borders = self.getBoxWidths().get("borders") width_int = parsedWidth - innerPaddings - borders return f'{width_int}px' diff --git a/mjml/elements/mj_section.py b/mjml/elements/mj_section.py index 03473d8..75c4a3b 100644 --- a/mjml/elements/mj_section.py +++ b/mjml/elements/mj_section.py @@ -1,10 +1,8 @@ - import re +import typing as t from collections import namedtuple from decimal import Decimal -from dotmap import DotMap - from ..helpers import parse_percentage, strip_unit, suffixCssClasses from ..lib import merge_dicts from ._base import BodyComponent @@ -19,7 +17,7 @@ class MjSection(BodyComponent): component_name = 'mj-section' @classmethod - def allowed_attrs(cls): + def allowed_attrs(cls) -> t.Dict[str, str]: return { 'background-color' : 'color', 'background-url' : 'string', @@ -136,7 +134,9 @@ def getBackground(self): def getBackgroundString(self): bg_pos = self.getBackgroundPosition() - return f'{bg_pos.posX} {bg_pos.posY}' + x = bg_pos.get("posX") + y = bg_pos.get("posY") + return f'{x} {y}' def getChildContext(self): box = self.getBoxWidths()['box'] @@ -188,12 +188,12 @@ def renderSimple(self): self.renderAfter() ]) - def getBackgroundPosition(self): + def getBackgroundPosition(self) -> t.Dict[str, t.Any]: pos = self.parseBackgroundPosition() - return DotMap({ + return { 'posX': self.getAttribute('background-position-x') or pos.x, 'posY': self.getAttribute('background-position-y') or pos.y, - }) + } def parseBackgroundPosition(self): posSplit = self.getAttribute('background-position').split(' ') @@ -337,8 +337,16 @@ def renderWithBackground(self, content): vY = 0 else: bgPos = self.getBackgroundPosition() - bgPosX = self._get_bg_percentage(bgPos.posX, ('left', 'center', 'right'), default='50%') - bgPosY = self._get_bg_percentage(bgPos.posY, ('top', 'center', 'bottom'), default='0%') + bgPosX = self._get_bg_percentage( + bgPos.get("posX"), + ('left', 'center', 'right'), + default='50%' + ) + bgPosY = self._get_bg_percentage( + bgPos.get("posY"), + ('top', 'center', 'bottom'), + default='0%' + ) # this logic is different when using repeat or no-repeat vX = self._calc_origin_pos_value(is_x=True, bg_pos=bgPosX) vY = self._calc_origin_pos_value(is_x=False, bg_pos=bgPosY) diff --git a/mjml/mjml2html.py b/mjml/mjml2html.py index cbef123..add23da 100644 --- a/mjml/mjml2html.py +++ b/mjml/mjml2html.py @@ -1,9 +1,8 @@ +import typing as t from io import BytesIO, StringIO from pathlib import Path, PurePath -from typing import List, Optional from bs4 import BeautifulSoup -from dotmap import DotMap from .core import initComponent from .core.registry import register_components, register_core_components @@ -11,8 +10,17 @@ from .lib import merge_dicts -def ignore_empty(values): - result = [] +if t.TYPE_CHECKING: + T = t.TypeVar("T") + + +class ParseResult(t.NamedTuple): + html: str + errors: t.Sequence[str] + + +def ignore_empty(values: t.Sequence[t.Optional["T"]]) -> t.Sequence["T"]: + result: t.List["T"] = [] for value in values: if value: result.append(value) @@ -20,7 +28,7 @@ def ignore_empty(values): def mjml_to_html(xml_fp_or_json, skeleton=None, template_dir=None, - custom_components: Optional[List]=None): + custom_components: t.Optional[t.List]=None) -> ParseResult: register_core_components() if isinstance(xml_fp_or_json, dict): @@ -53,7 +61,7 @@ def mjml_to_html(xml_fp_or_json, skeleton=None, template_dir=None, } # LATER: ability to override fonts via **options - globalDatas = DotMap({ + globalDatas: t.Dict[str, t.Any] = { 'backgroundColor' : None, 'breakpoint' : '480px', 'classes' : {}, @@ -69,12 +77,12 @@ def mjml_to_html(xml_fp_or_json, skeleton=None, template_dir=None, 'preview' : '', 'style' : [], 'title' : '', - }) + } # "validationLevel" is not used but available upstream - makes it easier to # match the line of code with the upstream sources. validationLevel = 'skip' # noqa: F841 - errors = [] + errors: t.List[str] = [] # LATER: optional validation mjBody = mjml_root('mj-body')[0] @@ -83,7 +91,8 @@ def mjml_to_html(xml_fp_or_json, skeleton=None, template_dir=None, assert len(mjHead) == 1 mjHead = mjHead[0] - def processing(node, context, parseMJML=None): + def processing(node: t.Optional[t.Any], context: t.Dict[str, t.Any], + parseMJML: t.Optional[t.Callable[[t.Any], t.Any]]=None) -> t.Optional[str]: if node is None: return None # LATER: upstream passes "parseMJML=identity" for head components @@ -97,6 +106,7 @@ def processing(node, context, parseMJML=None): return None if hasattr(component, 'handler'): return component.handler() + # TODO typing: this check is redundant, delete? if hasattr(component, 'render'): return component.render() raise AssertionError('should not reach this') @@ -120,19 +130,19 @@ def parse(_mjml, parentMjClass='', *, template_dir): attributesClasses = {} for css_class in classes: - mjClassValues = globalDatas.classes.get(css_class) + mjClassValues = globalDatas.get("classes").get(css_class) if mjClassValues: attributesClasses.update(mjClassValues) parent_mj_classes = ignore_empty(parentMjClass.split(' ')) def default_attr_classes(value): - return globalDatas.classesDefault.get(value, {}).get(tagName, {}) + return globalDatas.get("classesDefault").get(value, {}).get(tagName, {}) defaultAttributesForClasses = merge_dicts(*map(default_attr_classes, parent_mj_classes)) nextParentMjClass = attributes.get('mj-class', parentMjClass) _attrs_omit = omit(attributes, 'mj-class') _returned_attributes = merge_dicts( - globalDatas.defaultAttributes.get(tagName, {}), + globalDatas.get("defaultAttributes").get(tagName, {}), attributesClasses, defaultAttributesForClasses, _attrs_omit, @@ -147,7 +157,7 @@ def default_attr_classes(value): 'tagName': tagName, 'content': content, 'attributes': _returned_attributes, - 'globalAttributes': globalDatas.defaultAttributes.get('mj-all', {}).copy(), + 'globalAttributes': globalDatas.get("defaultAttributes").get('mj-all', {}).copy(), 'children': [], # will be set afterwards } _parse_mjml = lambda mjml: parse(mjml, nextParentMjClass, template_dir=template_dir) @@ -161,20 +171,20 @@ def default_attr_classes(value): return parse(mjml_element, template_dir=template_dir) def addHeadStyle(identifier, headStyle): - globalDatas.headStyle[identifier] = headStyle + globalDatas["headStyle"][identifier] = headStyle def addMediaQuery(className, parsedWidth, unit): width_str = f'{parsedWidth}{unit}' width_css = f'{{ width:{width_str} !important; max-width: {width_str}; }}' - globalDatas.mediaQueries[className] = width_css + globalDatas["mediaQueries"][className] = width_css def addComponentHeadSyle(headStyle): - globalDatas.componentsHeadStyle.append(headStyle) + globalDatas["componentsHeadStyle"].append(headStyle) def setBackgroundColor(color): - globalDatas.backgroundColor = color + globalDatas["backgroundColor"] = color - bodyHelpers = DotMap( + bodyHelpers = dict( addHeadStyle = addHeadStyle, addMediaQuery = addMediaQuery, addComponentHeadSyle = addComponentHeadSyle, @@ -200,15 +210,15 @@ def _head_data_add(attr, *params): assert len(param_values) == 1, 'shortcut in implementation' current_attr_value[param_key] = param_values[0] - headHelpers = DotMap( + headHelpers = dict( add = _head_data_add, ) - globalDatas.headRaw = processing(mjHead, headHelpers) + globalDatas["headRaw"] = processing(mjHead, headHelpers) content = processing(mjBody, bodyHelpers, applyAttributes) - if globalDatas.htmlAttributes: + if attrs := globalDatas.get("htmlAttributes"): contentSoup = BeautifulSoup(content, 'html.parser') - for selector, data in globalDatas.htmlAttributes.items(): + for selector, data in attrs.items(): for attrName, value in data.items(): for element in contentSoup.select(selector): element[attrName] = value or '' @@ -224,13 +234,13 @@ def _head_data_add(attr, *params): # LATER: upstream has also beautify # LATER: upstream has also minify - if len(globalDatas.inlineStyle) > 0: + if len(globalDatas.get("inlineStyle")) > 0: try: import css_inline except ImportError: raise ImportError('CSS inlining is an optional feature. Run `pip install -e ".[css_inlining]"` to install the required dependencies.') # noqa: E501 - extra_css = ''.join(globalDatas.inlineStyle) + extra_css = ''.join(globalDatas.get("inlineStyle")) inliner = css_inline.CSSInliner( extra_css=extra_css, inline_style_tags=False, @@ -242,10 +252,10 @@ def _head_data_add(attr, *params): content = mergeOutlookConditionnals(content) - return DotMap({ - 'html': content, - 'errors': errors, - }) + return ParseResult( + html=content, + errors=errors, + ) def _map_to_tuple(items, map_fn, filter_none=None): From 638e75c4acbd2f6d4c92429ba2aee217f25ef81f Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 22:46:37 +0200 Subject: [PATCH 04/14] typed width parser --- mjml/helpers/width_parser.py | 58 ++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/mjml/helpers/width_parser.py b/mjml/helpers/width_parser.py index 96d54c5..eab7a2d 100644 --- a/mjml/helpers/width_parser.py +++ b/mjml/helpers/width_parser.py @@ -1,19 +1,15 @@ - import re -from collections import namedtuple +import typing as t from .py_utils import strip_unit __all__ = ['widthParser'] -_WidthUnit = namedtuple('_WidthUnit', ('width', 'unit')) -class WidthUnit(_WidthUnit): - def __new__(cls, width, *, unit='px'): - if not unit: - unit = 'px' - return super().__new__(cls, width=width, unit=unit) +class WidthUnit(t.NamedTuple): + width: int + unit: str = "px" @property def parsedWidth(self): @@ -23,23 +19,33 @@ def __str__(self): return f'{self.width}{self.unit}' -unitRegex = re.compile(r'[\d.,]*(\D*)$') +def widthParser(width: str, parseFloatToInt: bool=True) -> WidthUnit: + pattern = re.compile(r'[\d\.\,]+(?P\D*)$') -def widthParser(width, parseFloatToInt=True): width_str = str(width) - match = unitRegex.search(width_str) - widthUnit = match.group(1) - if (widthUnit == '%') and not parseFloatToInt: - parser = float - else: - parser = int - width = strip_unit(width_str) - parsed_width = parser(width) - # LATER: somehow JS works differently here (as it does not have a strict - # type sytem). parseFloat() might return a number without fractional but - # python does. - width_int = int(width) - if parsed_width == width_int: - parsed_width = width_int - - return WidthUnit(width=parsed_width, unit=widthUnit) + + if (match := pattern.search(width_str)) is None: + raise ValueError(f"invalid width '{width}'") + + if (unit := match.groupdict().get("unit")) is None: + raise RuntimeError("something went wrong...") + + if not isinstance(unit, str): + raise TypeError(f"invalid unit '{unit}' ({type(unit)})") + + parser = float if (unit == '%') and not parseFloatToInt else int + + # TODO: the value returned from `strip_unit` is already an integer... + clean_width = strip_unit(width_str) + parsed_width = parser(clean_width) + + # NOTE: only relevant if we're parsing as float, and the value is an integer + numerator, denominator = parsed_width.as_integer_ratio() + + if denominator == 1: + parsed_width = numerator + + if not unit: + return WidthUnit(width=parsed_width) + + return WidthUnit(width=parsed_width, unit=unit) From b3ca0d60302041cca264ed3d6c01c08cef61703a Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:04:36 +0200 Subject: [PATCH 05/14] add typing for more helpers --- mjml/helpers/py_utils.py | 35 ++++++++++++++++++-------------- mjml/helpers/shorthand_parser.py | 12 +++++++---- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/mjml/helpers/py_utils.py b/mjml/helpers/py_utils.py index e9f2780..cd2de70 100644 --- a/mjml/helpers/py_utils.py +++ b/mjml/helpers/py_utils.py @@ -1,5 +1,5 @@ - import re +import typing as t from decimal import Decimal @@ -15,6 +15,8 @@ 'strip_unit', ] + +# TODO typing: figure out types def omit(attributes, keys): if isinstance(keys, str): keys = (keys, ) @@ -24,36 +26,39 @@ def omit(attributes, keys): _attrs.pop(key) return _attrs -def parse_float(value_str): - match = re.search(r'^([-+]?\d+(.\d+)?)*', value_str) +def parse_float(value: str) -> float: + if (match := re.search(r'^([-+]?\d+(.\d+)?)*', value)) is None: + raise ValueError(f"could not parse float from '{value}'") return float(match.group(1)) -def parse_int(value_str): - if isinstance(value_str, int): - return value_str - match = re.search(r'^([-+]?\d+)*', value_str) +def parse_int(value: str) -> int: + if (match := re.search(r'^([-+]?\d+)*', value)) is None: + raise ValueError(f"could not parse int from '{value}'") return int(match.group(1)) -def parse_percentage(value_str): - match = re.search(r'^(\d+(\.\d+)?)%$', value_str) +def parse_percentage(value: str) -> Decimal: + if (match := re.search(r'^(\d+(\.\d+)?)%$', value)) is None: + raise ValueError(f"could not parse decimal from '{value}'") return Decimal(match.group(1)) -def strip_unit(value_str): - match = re.search(r'^(\d+).*', value_str) +# TODO: fix if this should support decimals/floats as well +def strip_unit(value: str) -> int: + if (match := re.search(r'^(\d+).*', value)) is None: + raise ValueError(f"could not strip unit from '{value}'") return int(match.group(1)) -def is_nil(v): +def is_nil(v: t.Optional[t.Any]) -> bool: return (v is None) -def is_not_nil(v): +def is_not_nil(v: t.Optional[t.Any]) -> bool: return not is_nil(v) -def is_empty(v): +def is_empty(v: t.Optional[t.Sequence[t.Any]]) -> bool: if v is None: return True elif hasattr(v, 'strip'): return not bool(v.strip()) return not bool(v) -def is_not_empty(v): +def is_not_empty(v: t.Optional[t.Sequence[t.Any]]) -> bool: return not is_empty(v) diff --git a/mjml/helpers/shorthand_parser.py b/mjml/helpers/shorthand_parser.py index 5eaddff..8dc82bd 100644 --- a/mjml/helpers/shorthand_parser.py +++ b/mjml/helpers/shorthand_parser.py @@ -1,12 +1,16 @@ - import re +import typing as t from .py_utils import parse_int +if t.TYPE_CHECKING: + _Direction = t.Literal["top", "bottom", "left", "right"] + + __all__ = ['shorthandParser', 'borderParser'] -def shorthandParser(cssValue, direction): +def shorthandParser(cssValue: str, direction: "_Direction") -> int: splittedCssValue = cssValue.split(' ') top = 0 @@ -29,11 +33,11 @@ def shorthandParser(cssValue, direction): directions = {'top': top, 'bottom': bottom, 'left': left, 'right': right} - value_int = splittedCssValue[directions[direction]] or 0 + value_int = splittedCssValue[directions[direction]] or "0" return parse_int(value_int) -def borderParser(border): +def borderParser(border: str) -> int: border_regex = re.compile(r'(?:(?:^| )(\d+))') match = border_regex.search(border) # Upstream does not have to deal with this case as "parseInt(undefined, 10)" From f33abf69f1da8612c29d142e4f458ad795dd89fb Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:06:05 +0200 Subject: [PATCH 06/14] fix redundant parsing --- mjml/elements/mj_divider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mjml/elements/mj_divider.py b/mjml/elements/mj_divider.py index f09ebdc..4f2eeaf 100644 --- a/mjml/elements/mj_divider.py +++ b/mjml/elements/mj_divider.py @@ -63,7 +63,7 @@ def getOutlookWidth(self): parsedWidth, unit = widthParser(width) if unit == '%': - px = (parse_int(containerWidth) * parse_int(parsedWidth)) / 100 - paddingSize + px = (parse_int(containerWidth) * parsedWidth) / 100 - paddingSize return f'{px}px' elif unit == 'px': return width From 6321eccb2f76422698220c812a9b5c6fffa43a9f Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:12:48 +0200 Subject: [PATCH 07/14] create dedicated file for types --- mjml/_types.py | 4 ++++ mjml/helpers/shorthand_parser.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 mjml/_types.py diff --git a/mjml/_types.py b/mjml/_types.py new file mode 100644 index 0000000..9fab282 --- /dev/null +++ b/mjml/_types.py @@ -0,0 +1,4 @@ +import typing as t + + +_Direction = t.Literal["top", "bottom", "left", "right"] diff --git a/mjml/helpers/shorthand_parser.py b/mjml/helpers/shorthand_parser.py index 8dc82bd..a6889ae 100644 --- a/mjml/helpers/shorthand_parser.py +++ b/mjml/helpers/shorthand_parser.py @@ -5,7 +5,7 @@ if t.TYPE_CHECKING: - _Direction = t.Literal["top", "bottom", "left", "right"] + from mjml._types import _Direction __all__ = ['shorthandParser', 'borderParser'] From a657f0d4421d71c7d3b1cc930ac164cbe2acb5a7 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:22:26 +0200 Subject: [PATCH 08/14] typing for body component --- mjml/elements/_base.py | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/mjml/elements/_base.py b/mjml/elements/_base.py index 190acfd..e9c8570 100644 --- a/mjml/elements/_base.py +++ b/mjml/elements/_base.py @@ -1,3 +1,4 @@ +import itertools import typing as t from ..core import Component, initComponent @@ -6,15 +7,22 @@ from ..lib import merge_dicts +if t.TYPE_CHECKING: + from mjml._types import _Direction + + __all__ = [ 'BodyComponent', ] + class BodyComponent(Component): - def render(self): + def render(self) -> str: raise NotImplementedError(f'{self.__cls__.__name__} should override ".render()"') - def getShorthandAttrValue(self, attribute, direction, attr_with_direction=True): + def getShorthandAttrValue(self, + attribute: str, direction: "_Direction", + attr_with_direction: bool=True) -> int: if attr_with_direction: mjAttributeDirection = self.getAttribute(f'{attribute}-{direction}') else: @@ -27,7 +35,7 @@ def getShorthandAttrValue(self, attribute, direction, attr_with_direction=True): return 0 return shorthandParser(mjAttribute, direction) - def getShorthandBorderValue(self, direction): + def getShorthandBorderValue(self, direction: "_Direction") -> int: borderDirection = direction and self.getAttribute(f'border-{direction}') border = self.getAttribute('border') return borderParser(borderDirection or border or '0') @@ -47,9 +55,8 @@ def getBoxWidths(self) -> t.Dict[str, t.Any]: } # js: htmlAttributes(attributes) - def html_attrs(self, **attrs): - def _to_str(kv): - key, value = kv + def html_attrs(self, **attrs: t.Any) -> str: + def _to_str(key: str, value: t.Any) -> t.Optional[str]: if key == 'style': value = self.styles(value) elif key in ['class_', 'for_']: @@ -59,34 +66,39 @@ def _to_str(kv): if value is None: return None return f'{key}="{value}"' - serialized_attrs = map(_to_str, attrs.items()) + serialized_attrs = itertools.starmap(_to_str, attrs.items()) return ' '.join(filter(None, serialized_attrs)) # js: getStyles() - def get_styles(self): + def get_styles(self) -> t.Dict[str, t.Any]: return {} # js: styles(styles) - def styles(self, key=None): - _styles = None + def styles(self, key: t.Optional[t.Any]=None) -> str: + _styles: t.Optional[t.Dict[str, t.Any]] = None + if key and isinstance(key, str): _styles_dict = self.get_styles() keys = key.split('.') _styles = _styles_dict.get(keys[0]) if len(keys) > 1: + # TODO typing: fix + if not _styles: + raise RuntimeError() _styles = _styles.get(keys[1]) if _styles and not isinstance(_styles, dict): raise ValueError(f'key={key}') elif key: # predefined dict _styles = key + if not _styles: _styles = {} - def serializer(kv): - k, v = kv + def serializer(k: str, v: t.Any) -> t.Optional[str]: return f'{k}:{v}' if is_not_empty(v) else None - style_attr_strs = filter(None, map(serializer, _styles.items())) + + style_attr_strs = filter(None, itertools.starmap(serializer, _styles.items())) style_str = ';'.join(style_attr_strs) return style_str From 8671ef8d5b747e254062b328043034a558b86561 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:23:26 +0200 Subject: [PATCH 09/14] fix invalid class attribute --- mjml/elements/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mjml/elements/_base.py b/mjml/elements/_base.py index e9c8570..7f2a61b 100644 --- a/mjml/elements/_base.py +++ b/mjml/elements/_base.py @@ -18,7 +18,7 @@ class BodyComponent(Component): def render(self) -> str: - raise NotImplementedError(f'{self.__cls__.__name__} should override ".render()"') + raise NotImplementedError(f'{self.__class__.__name__} should override ".render()"') def getShorthandAttrValue(self, attribute: str, direction: "_Direction", From 00a68da2f1db3e46c1eda5fe18b9f59b31f60590 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Mon, 1 Jul 2024 23:30:29 +0200 Subject: [PATCH 10/14] further typing of body component --- mjml/elements/_base.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mjml/elements/_base.py b/mjml/elements/_base.py index 7f2a61b..6c2ab9e 100644 --- a/mjml/elements/_base.py +++ b/mjml/elements/_base.py @@ -102,8 +102,10 @@ def serializer(k: str, v: t.Any) -> t.Optional[str]: style_str = ';'.join(style_attr_strs) return style_str - def renderChildren(self, childrens=None, props=None, renderer=None, - attributes=None, rawXML=False): + # TODO typing: finish rest of type annotations + def renderChildren(self, childrens=None, props=None, + renderer: t.Optional[t.Callable[[Component], str]]=None, + attributes=None, rawXML=False) -> str: if not props: props = {} if not renderer: @@ -128,8 +130,8 @@ def renderChildren(self, childrens=None, props=None, renderer=None, # child => !find(rawComponents, c => c.getTagName() === child.tagName), #).length raw_tag_names = set() - for tag_name, component in components.items(): - if component.isRawElement(): + for tag_name, component_cls in components.items(): + if component_cls.isRawElement(): raw_tag_names.add(tag_name) is_raw_element = lambda c: (c['tagName'] in raw_tag_names) From 3cca2c60a4ee539db4f32c5b47f4a2d72ba4adfc Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Tue, 2 Jul 2024 00:19:50 +0200 Subject: [PATCH 11/14] call handler only if head component --- mjml/core/api.py | 3 +++ mjml/mjml2html.py | 20 +++++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/mjml/core/api.py b/mjml/core/api.py index 4a0ce7c..081d8db 100644 --- a/mjml/core/api.py +++ b/mjml/core/api.py @@ -93,5 +93,8 @@ def get_attr(self, name: str, *, missing_ok: bool=False) -> t.Optional[t.Any]: return self.attrs.get(name) getAttribute = get_attr + def handler(self) -> t.Optional[str]: + return None + def render(self) -> str: return '' diff --git a/mjml/mjml2html.py b/mjml/mjml2html.py index add23da..1782b32 100644 --- a/mjml/mjml2html.py +++ b/mjml/mjml2html.py @@ -4,6 +4,8 @@ from bs4 import BeautifulSoup +from mjml.elements.head._head_base import HeadComponent + from .core import initComponent from .core.registry import register_components, register_core_components from .helpers import json_to_xml, mergeOutlookConditionnals, omit, skeleton_str as default_skeleton @@ -42,7 +44,9 @@ def mjml_to_html(xml_fp_or_json, skeleton=None, template_dir=None, template_dir = Path(xml_fp.name).parent mjml_doc = BeautifulSoup(xml_fp, 'html.parser') - mjml_root = mjml_doc.mjml + + if (mjml_root := mjml_doc.mjml) is None: + raise ValueError(f"could not parse '{xml_fp.name}'") skeleton_path = skeleton if skeleton_path: @@ -104,17 +108,19 @@ def processing(node: t.Optional[t.Any], context: t.Dict[str, t.Any], component = initComponent(name=node_tag, **initialDatas) if not component: return None - if hasattr(component, 'handler'): + if isinstance(component, HeadComponent): return component.handler() # TODO typing: this check is redundant, delete? if hasattr(component, 'render'): return component.render() raise AssertionError('should not reach this') - def applyAttributes(mjml_element): + def applyAttributes(mjml_element: t.Any) -> t.Dict[str, t.Any]: if len(mjml_element) == 0: return {} - def parse(_mjml, parentMjClass='', *, template_dir): + + # TODO typing: figure out proper annotations + def parse(_mjml, parentMjClass: str='', *, template_dir: str) -> t.Any: tagName = _mjml.name is_comment = not isinstance(tagName, str) if is_comment: @@ -129,14 +135,18 @@ def parse(_mjml, parentMjClass='', *, template_dir): content = _mjml.decode_contents() attributesClasses = {} + for css_class in classes: mjClassValues = globalDatas.get("classes").get(css_class) if mjClassValues: attributesClasses.update(mjClassValues) parent_mj_classes = ignore_empty(parentMjClass.split(' ')) - def default_attr_classes(value): + + # TODO typing: figure out proper annotations + def default_attr_classes(value: t.Any) -> t.Any: return globalDatas.get("classesDefault").get(value, {}).get(tagName, {}) + defaultAttributesForClasses = merge_dicts(*map(default_attr_classes, parent_mj_classes)) nextParentMjClass = attributes.get('mj-class', parentMjClass) From 2797efb618eb40284165c3b1265d88a495fd1584 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Tue, 2 Jul 2024 00:20:41 +0200 Subject: [PATCH 12/14] typing for head elements --- mjml/elements/head/_head_base.py | 5 ++++- mjml/elements/head/mj_attributes.py | 12 +++++++++--- mjml/elements/head/mj_breakpoint.py | 11 ++++++++--- mjml/elements/head/mj_font.py | 11 ++++++++--- mjml/elements/head/mj_head.py | 9 +++++++-- mjml/elements/head/mj_html_attributes.py | 8 ++++++-- mjml/elements/head/mj_preview.py | 8 ++++++-- mjml/elements/head/mj_style.py | 11 ++++++++--- mjml/elements/head/mj_title.py | 8 ++++++-- 9 files changed, 62 insertions(+), 21 deletions(-) diff --git a/mjml/elements/head/_head_base.py b/mjml/elements/head/_head_base.py index db6f95d..c2ba81a 100644 --- a/mjml/elements/head/_head_base.py +++ b/mjml/elements/head/_head_base.py @@ -1,12 +1,15 @@ +import typing as t from mjml.core import Component, initComponent __all__ = ['HeadComponent'] + class HeadComponent(Component): + # TODO typing: figure out proper type annotations def handlerChildren(self): - def handle_children(children): + def handle_children(children: t.Dict[str, t.Any]) -> t.Optional[str]: tagName = children['tagName'] component = initComponent( name = tagName, diff --git a/mjml/elements/head/mj_attributes.py b/mjml/elements/head/mj_attributes.py index bed3d47..70c5a63 100644 --- a/mjml/elements/head/mj_attributes.py +++ b/mjml/elements/head/mj_attributes.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from mjml.helpers import omit @@ -6,12 +9,15 @@ __all__ = ['MjAttributes'] + class MjAttributes(HeadComponent): - component_name = 'mj-attributes' + component_name: t.ClassVar[str] = 'mj-attributes' - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] - _children = self.props.get("children") + if (_children := self.props.get("children")) is None: + return None for child in _children: tagName = child['tagName'] diff --git a/mjml/elements/head/mj_breakpoint.py b/mjml/elements/head/mj_breakpoint.py index 6a8d54f..d12e1d7 100644 --- a/mjml/elements/head/mj_breakpoint.py +++ b/mjml/elements/head/mj_breakpoint.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -6,14 +9,16 @@ class MjBreakpoint(HeadComponent): - component_name = 'mj-breakpoint' + component_name: t.ClassVar[str] = 'mj-breakpoint' + @te.override @classmethod - def allowed_attrs(cls): + def allowed_attrs(cls) -> t.Dict[str, str]: return { 'width': 'unit(px)', } - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] add('breakpoint', self.getAttribute('width')) diff --git a/mjml/elements/head/mj_font.py b/mjml/elements/head/mj_font.py index 8661edb..2469d7b 100644 --- a/mjml/elements/head/mj_font.py +++ b/mjml/elements/head/mj_font.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -6,15 +9,17 @@ class MjFont(HeadComponent): - component_name = 'mj-font' + component_name: t.ClassVar[str] = 'mj-font' + @te.override @classmethod - def allowed_attrs(cls): + def allowed_attrs(cls) -> t.Dict[str, str]: return { 'href' : 'string', 'name' : 'string', } - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] add('fonts', self.getAttribute('name'), self.getAttribute('href')) diff --git a/mjml/elements/head/mj_head.py b/mjml/elements/head/mj_head.py index 36c4331..3daea30 100644 --- a/mjml/elements/head/mj_head.py +++ b/mjml/elements/head/mj_head.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -5,7 +8,9 @@ __all__ = ['MjHead'] class MjHead(HeadComponent): - component_name = 'mj-head' + component_name: t.ClassVar[str] = 'mj-head' - def handler(self): + @te.override + def handler(self) -> t.Optional[str]: + # TODO typing: fix return self.handlerChildren() diff --git a/mjml/elements/head/mj_html_attributes.py b/mjml/elements/head/mj_html_attributes.py index 93d953e..82ede27 100644 --- a/mjml/elements/head/mj_html_attributes.py +++ b/mjml/elements/head/mj_html_attributes.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -5,9 +8,10 @@ __all__ = ['MjHtmlAttributes'] class MjHtmlAttributes(HeadComponent): - component_name = 'mj-html-attributes' + component_name: t.ClassVar[str] = 'mj-html-attributes' - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] _children = self.props.get("children") diff --git a/mjml/elements/head/mj_preview.py b/mjml/elements/head/mj_preview.py index 2af8ce5..1919f54 100644 --- a/mjml/elements/head/mj_preview.py +++ b/mjml/elements/head/mj_preview.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -6,8 +9,9 @@ class MjPreview(HeadComponent): - component_name = 'mj-preview' + component_name: t.ClassVar[str] = 'mj-preview' - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] add('preview', self.getContent()) diff --git a/mjml/elements/head/mj_style.py b/mjml/elements/head/mj_style.py index 1da2024..c334f3e 100644 --- a/mjml/elements/head/mj_style.py +++ b/mjml/elements/head/mj_style.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -5,15 +8,17 @@ __all__ = ['MjStyle'] class MjStyle(HeadComponent): - component_name = 'mj-style' + component_name: t.ClassVar[str] = 'mj-style' + @te.override @classmethod - def default_attrs(cls): + def default_attrs(cls) -> t.Dict[str, str]: return { 'inline': '', } - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] inline_attr = 'inlineStyle' if (self.get_attr('inline') == 'inline') else 'style' html_str = self.getContent() diff --git a/mjml/elements/head/mj_title.py b/mjml/elements/head/mj_title.py index ff89e96..355c610 100644 --- a/mjml/elements/head/mj_title.py +++ b/mjml/elements/head/mj_title.py @@ -1,3 +1,6 @@ +import typing as t + +import typing_extensions as te from ._head_base import HeadComponent @@ -5,8 +8,9 @@ __all__ = ['MjTitle'] class MjTitle(HeadComponent): - component_name = 'mj-title' + component_name: t.ClassVar[str] = 'mj-title' - def handler(self): + @te.override + def handler(self) -> None: add = self.context['add'] add('title', self.getContent()) From 4fbc0afb170904cc1f2e34c7dfd4649844fb58e8 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Tue, 2 Jul 2024 19:42:00 +0200 Subject: [PATCH 13/14] add typecheck workflow --- .github/workflows/typecheck.yml | 56 +++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .github/workflows/typecheck.yml diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml new file mode 100644 index 0000000..0253203 --- /dev/null +++ b/.github/workflows/typecheck.yml @@ -0,0 +1,56 @@ +name: typecheck + +on: + push: + branches: + - main + - typing + pull_request: + branches: + - main + +jobs: + mypy: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: + ["3.8", "3.9", "3.10", "3.11", "3.12", "pypy-3.9", "pypy-3.10"] + option: ["", "--strict"] + include: + - python-version: "3.8" + version-name: "3.8" + - python-version: "3.9" + version-name: "3.9" + - python-version: "3.10" + version-name: "3.10" + - python-version: "3.11" + version-name: "3.11" + - python-version: "3.12" + version-name: "3.12" + - python-version: "pypy-3.9" + version-name: "3.9" + - python-version: "pypy-3.10" + version-name: "3.10" + + steps: + - name: Check out repository code + uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + # TODO typing: fix + - name: Install dependencies + run: | + pip install "mypy==1.10.1" + pip install -e .[testing] + pip install -e .[css_inlining] + pip install types-docopt types-beautifulsoup4 + + - name: Run mypy ${{ matrix.option }} + run: | + mypy ${{ matrix.option }} --python-version="${{ matrix.version-name }}" mjml + if: ${{ !cancelled() }} From 5801e93ee58883985717f2e7e8d6c85a9f5443a6 Mon Sep 17 00:00:00 2001 From: Dan Lindholm Date: Tue, 2 Jul 2024 20:45:47 +0200 Subject: [PATCH 14/14] add typing_extensions to required installs --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index e3cca47..50b283d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -44,6 +44,7 @@ install_requires = dotmap docopt jinja2 + typing_extensions # TODO typing: move elsewhere if possible scripts = mjml/scripts/mjml-html-compare