jdrouet / mrml

Implementation of mjml in rust
MIT License
344 stars 22 forks source link

feat(mrml-core): add `Fragment` element #437

Open JadedBlueEyes opened 3 months ago

JadedBlueEyes commented 3 months ago

This component effectively just acts as a collection of children. However, when rendered, they behave as if directly under the parent element. This is useful for passing around reusable components, etc, in Rust code.

Right now, it is not actually possible to parse a Fragment. Instead, they must be constructed by Rust code. This is because of some non-configurable validation in xmlparser: https://github.com/RazrFalcon/xmlparser/blob/e54c2768f20814140c11e6c92f94ee74bfd54808/src/stream.rs#L432

I'm using this in mrmx, as it makes it quite a bit more pleasant to use.

This might have a slight performance impact, as fn get_children allocates a Vector to help fold all children and nested Fragment children into one array. Before, this was directly calling .iter() on self.element.children. However, local benchmarking doesn't find any significant difference. I'm very much open to a different solution, though!


     Running benches/basic.rs (C:\Users\jade\.cache\cargo-target\release\deps\basic-9ef9e838d0ba5c55.exe)
Benchmarking render button: Collecting 100 samples in estimated 5.0707 s 
                        time:   [16.882 µs 17.014 µs 17.161 µs]
                        change: [-3.1318% -0.5849% +1.9841%] (p = 0.66 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

     Running benches/template.rs (C:\Users\jade\.cache\cargo-target\release\deps\template-7a43bc935c493ad3.exe)
Benchmarking amario: Collecting 100 samples in estimated 5.4222 s (10k itera
                        time:   [525.70 µs 529.71 µs 534.01 µs]
                        change: [-3.9368% -1.2313% +1.1003%] (p = 0.37 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe
jdrouet commented 3 months ago

Moving your PR to draft considering the amount of commented code and the points mentioned.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 74.18398% with 87 lines in your changes missing coverage. Please review.

Project coverage is 92.74%. Comparing base (5311e58) to head (726ef4e). Report is 61 commits behind head on main.

:exclamation: Current head 726ef4e differs from pull request most recent head e67713a

Please upload reports for the commit e67713a to get more accurate results.

Files Patch % Lines
packages/mrml-core/src/fragment/render.rs 32.20% 40 Missing :warning:
packages/mrml-core/src/fragment/parse.rs 0.00% 20 Missing :warning:
packages/mrml-core/src/fragment/print.rs 0.00% 15 Missing :warning:
packages/mrml-core/src/mj_body/children.rs 0.00% 2 Missing :warning:
packages/mrml-core/src/mj_accordion/render.rs 92.30% 1 Missing :warning:
packages/mrml-core/src/mj_carousel/render.rs 94.73% 1 Missing :warning:
packages/mrml-core/src/mj_head/mod.rs 94.44% 1 Missing :warning:
packages/mrml-core/src/mj_include/body/parse.rs 80.00% 1 Missing :warning:
packages/mrml-core/src/mj_include/body/render.rs 92.85% 1 Missing :warning:
packages/mrml-core/src/mj_include/head/parse.rs 83.33% 1 Missing :warning:
... and 4 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #437 +/- ## ========================================== - Coverage 92.89% 92.74% -0.15% ========================================== Files 227 227 Lines 12203 13153 +950 ========================================== + Hits 11336 12199 +863 - Misses 867 954 +87 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.