Skip to content

Conversation

@justinfagnani
Copy link
Collaborator

@justinfagnani justinfagnani commented May 2, 2024

Fixes #4636

This adds a mathml template tag to enable writing MathML fragment templates, just like how we have the svg template tag for SVG fragment templates.

The change adds another TemplateResult type for MATHML, and just creates a <math> wrapper element for fragment templates like we do for SVG.

I had to update the @open-wc/testing dependency of the starter kits because 3.x had hard-coded the template result types. 4.x works with the new type. So there is the possibility that that this will break type-checking of other libraries that hard-code the types. Hopefully they can upgrade quickly if they need to. Our policy is to not consider type-only breaks as semver major, so this change is semver minor.

cc @lukewarlow @bkardell 😄

@changeset-bot
Copy link

changeset-bot bot commented May 2, 2024

🦋 Changeset detected

Latest commit: 2b78f7e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
lit-html Minor
lit Minor
lit-element Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2024

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +7% (-0.27ms - +0.77ms)
    this-change vs tip-of-tree

render

  • this-change: 44.07ms - 46.39ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +5% (-0.19ms - +0.91ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-0.57ms - +0.25ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-0.69ms - +0.20ms)
    this-change vs tip-of-tree

update

  • this-change: 447.87ms - 459.02ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +9% (-0.97ms - +3.23ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-0.94ms - +0.70ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +0% (-3.11ms - +1.17ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 453.16ms - 457.74ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-2.84ms - +3.10ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
44.07ms - 46.39ms-

update

VersionAvg timevs
447.87ms - 459.02ms-

update-reflect

VersionAvg timevs
453.16ms - 457.74ms-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
18.07ms - 18.90ms-unsure 🔍
-1% - +5%
-0.19ms - +0.91ms
unsure 🔍
-2% - +4%
-0.38ms - +0.76ms
tip-of-tree
tip-of-tree
17.77ms - 18.48msunsure 🔍
-5% - +1%
-0.91ms - +0.19ms
-unsure 🔍
-4% - +2%
-0.69ms - +0.35ms
previous-release
previous-release
17.91ms - 18.68msunsure 🔍
-4% - +2%
-0.76ms - +0.38ms
unsure 🔍
-2% - +4%
-0.35ms - +0.69ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
34.79ms - 37.75ms-unsure 🔍
-3% - +9%
-0.97ms - +3.23ms
unsure 🔍
-2% - +9%
-0.80ms - +3.08ms
tip-of-tree
tip-of-tree
33.66ms - 36.63msunsure 🔍
-9% - +3%
-3.23ms - +0.97ms
-unsure 🔍
-6% - +6%
-1.93ms - +1.95ms
previous-release
previous-release
33.88ms - 36.39msunsure 🔍
-8% - +2%
-3.08ms - +0.80ms
unsure 🔍
-6% - +6%
-1.95ms - +1.93ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
10.70ms - 11.46ms-unsure 🔍
-3% - +7%
-0.27ms - +0.77ms
unsure 🔍
-3% - +6%
-0.33ms - +0.63ms
tip-of-tree
tip-of-tree
10.47ms - 11.19msunsure 🔍
-7% - +2%
-0.77ms - +0.27ms
-unsure 🔍
-5% - +3%
-0.56ms - +0.37ms
previous-release
previous-release
10.63ms - 11.22msunsure 🔍
-6% - +3%
-0.63ms - +0.33ms
unsure 🔍
-3% - +5%
-0.37ms - +0.56ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
32.80ms - 33.44ms-unsure 🔍
-2% - +1%
-0.57ms - +0.25ms
unsure 🔍
-1% - +1%
-0.47ms - +0.45ms
tip-of-tree
tip-of-tree
33.02ms - 33.54msunsure 🔍
-1% - +2%
-0.25ms - +0.57ms
-unsure 🔍
-1% - +2%
-0.28ms - +0.57ms
previous-release
previous-release
32.79ms - 33.47msunsure 🔍
-1% - +1%
-0.45ms - +0.47ms
unsure 🔍
-2% - +1%
-0.57ms - +0.28ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
64.03ms - 65.08ms-unsure 🔍
-1% - +1%
-0.94ms - +0.70ms
unsure 🔍
-2% - +1%
-1.14ms - +0.78ms
tip-of-tree
tip-of-tree
64.05ms - 65.30msunsure 🔍
-1% - +1%
-0.70ms - +0.94ms
-unsure 🔍
-2% - +1%
-1.08ms - +0.96ms
previous-release
previous-release
63.93ms - 65.54msunsure 🔍
-1% - +2%
-0.78ms - +1.14ms
unsure 🔍
-1% - +2%
-0.96ms - +1.08ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
29.56ms - 30.17ms-unsure 🔍
-2% - +1%
-0.69ms - +0.20ms
unsure 🔍
-3% - +1%
-0.81ms - +0.17ms
tip-of-tree
tip-of-tree
29.79ms - 30.43msunsure 🔍
-1% - +2%
-0.20ms - +0.69ms
-unsure 🔍
-2% - +1%
-0.57ms - +0.43ms
previous-release
previous-release
29.80ms - 30.57msunsure 🔍
-1% - +3%
-0.17ms - +0.81ms
unsure 🔍
-1% - +2%
-0.43ms - +0.57ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
451.62ms - 454.45ms-unsure 🔍
-1% - +0%
-3.11ms - +1.17ms
unsure 🔍
-1% - +0%
-2.59ms - +1.97ms
tip-of-tree
tip-of-tree
452.39ms - 455.61msunsure 🔍
-0% - +1%
-1.17ms - +3.11ms
-unsure 🔍
-0% - +1%
-1.74ms - +3.07ms
previous-release
previous-release
451.55ms - 455.13msunsure 🔍
-0% - +1%
-1.97ms - +2.59ms
unsure 🔍
-1% - +0%
-3.07ms - +1.74ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
490.73ms - 495.03ms-unsure 🔍
-1% - +1%
-2.84ms - +3.10ms
unsure 🔍
-0% - +1%
-2.02ms - +3.34ms
tip-of-tree
tip-of-tree
490.70ms - 494.80msunsure 🔍
-1% - +1%
-3.10ms - +2.84ms
-unsure 🔍
-0% - +1%
-2.07ms - +3.13ms
previous-release
previous-release
490.62ms - 493.82msunsure 🔍
-1% - +0%
-3.34ms - +2.02ms
unsure 🔍
-1% - +0%
-3.13ms - +2.07ms
-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2024

The size of lit-html.js and lit-core.min.js are as expected.

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, but maybe some cursory SSR tests would be nice?
e.g.

'ChildPart accepts TemplateResult with SVG type': {

test('svg fragment template', async () => {

Also, should there be an unsafeMath() directive, like unsafeSVG()?

@justinfagnani
Copy link
Collaborator Author

@augustjk Thanks! I added unsafeMath() and SSR tests

@justinfagnani
Copy link
Collaborator Author

There's a small question of the name of the template tag. I chose math because I think it's a lot easier to read and write then mathml.

html, svg, and css are more clear about what their content are than just math, and that might help with built-in language support in tools. For instance, mathml might conceivable trigger XML or even MathML specific highlighting. math might not - though we can highlight it in our tools.

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Regarding the tag name, I have a preference for mathml as the full name of the markup language, but not strong enough to block. Maybe an online poll on discord or twitter?

If we do change, I suppose the unsafe directive name should change to unsafeMathML too.

@justinfagnani
Copy link
Collaborator Author

Regarding the tag name, I have a preference for mathml

Let's ask Discord before merging.

It's not unchangeable at least. Just a couple of byte cost to export two names.

@lukewarlow
Copy link

Math being the root element name for the MathML namespace makes me lean towards math as the tagged template name too fwiw.

@justinfagnani
Copy link
Collaborator Author

Math being the root element name for the MathML namespace makes me lean towards math as the tagged template name too fwiw.

That's a good point. <svg> -> svg, <math> -> math

@justinfagnani
Copy link
Collaborator Author

Discord seems to be leaning heavily towards mathml

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change from math to mathml LGTM

@augustjk
Copy link
Member

@justinfagnani Do we also need a static version of mathml tag function?

@justinfagnani
Copy link
Collaborator Author

@justinfagnani Do we also need a static version of mathml tag function?

done!

@justinfagnani justinfagnani merged commit feccc1b into main Jul 3, 2024
@justinfagnani justinfagnani deleted the mathml branch July 3, 2024 02:22
@justinfagnani justinfagnani changed the title [lit-html] Add MathML support with the math template tag [lit-html] Add MathML support with the mathml template tag Jul 3, 2024
@JosefJezek JosefJezek mentioned this pull request Jul 26, 2024
4 tasks
@lit-robot lit-robot mentioned this pull request Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[lit-html] MathML suppport

4 participants