-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lit-html] Add MathML support with the math template tag #4637
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 3e852c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
📊 Tachometer Benchmark ResultsSummarynop-update
render
update
update-reflect
Resultsthis-change
render
update
update-reflect
this-change, tip-of-tree, previous-release
render
update
nop-update
this-change, tip-of-tree, previous-release
render
update
this-change, tip-of-tree, previous-release
render
update
update-reflect
|
The size of lit-html.js and lit-core.min.js are as expected. |
There was a problem hiding this 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()
?
Co-authored-by: Augustine Kim <augustinekim@google.com>
@augustjk Thanks! I added |
There's a small question of the name of the template tag. I chose
|
There was a problem hiding this 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.
Let's ask Discord before merging. It's not unchangeable at least. Just a couple of byte cost to export two names. |
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. |
Discord seems to be leaning heavily towards |
There was a problem hiding this 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
Fixes #4636
This adds a
math
template tag to enable writing MathML fragment templates, just like how we have thesvg
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 😄