DefinitelyTyped
Add MathML Support to `@types/react`
This adds support to React for MathML as discussed previously.
- [x] Use a meaningful title for the pull request. Include the name of the package modified.
- [x] Test the change in your own code. (Compile and run.)
- [x] Add or edit tests to reflect the change.
- [x] Follow the advice from the readme.
- [x] Avoid common mistakes.
- [x] Run
pnpm test <package to test>.
If changing an existing definition:
- [x] Provide a URL to documentation or source code which provides context for the suggested changes: https://developer.mozilla.org/en-US/docs/Web/MathML
- [ ] If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the
package.json.
Notes
In initial discussion, it was brought up that:
If it works at runtime, we should support it with types.
MathML is still newly supported by modern browsers. As a result, the browser support for some elements – and especially for some element attributes – is mixed. In some cases, elements have been deprecated and/or have had support removed. In other cases, the support across browsers is "non-standard", as MDN puts it. For example, various alignment and styling attributes on the mtable element are not supported in Chrome (v126 at the time of this writing), but are supported in Firefox. This can be seen by previewing the example from MDN:
<mtable frame="solid" rowlines="solid" align="axis 3">
(Chrome):
(Firefox):
~~In regards to the spectrum of support, the approach initially taken in this PR is as follows:~~
- ~~All elements MDN lists, except for those marked as deprecated and/or non-standard, have relevant types added in this PR.~~
- ~~All attributes MDN lists for a given element, except for those marked as deprecated, have relevant types added in this PR. Note that types for "non-standard" attributes have been added in this PR.~~
~~This approach was taken to reduce the initial blast radius of the initial MathML support with the intent that more detailed elements/attributes with further afield support could be added if the community finds a need to reach for them. That being said, I'm happy to discuss this more and adjust to whatever the maintainers of these types see as the best approach.~~
The difference in seems to come from the difference between MathML and MathML Core from the Math WG:
As MDN puts summarizes:
MDN uses MathML Core as a reference specification but, due to an erratic standardization history, legacy MathML features may still show up in existing implementations and web content.
and
Note: It is highly recommended that developers and authors switch to MathML Core, perhaps relying on other web technologies to cover missing use cases. The Math WG is maintaining a set of MathML polyfills to facilitate that transition.
The approach taken in this PR is to use the MathML Core specification. Note: Updates still need to be made to fully support this specification.
@louisscruz Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.
This is a live comment that I will keep updated.
1 package in this PR
Code Reviews
Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.
You can test the changes of this PR in the Playground.
Status
- ✅ No merge conflicts
- ✅ Continuous integration tests have passed
- 🕐 Most recent commit is approved by a DT maintainer
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.
Inactive
This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!
Diagnostic Information: What the bot saw about this PR
{
"type": "info",
"now": "-",
"pr_number": 69986,
"author": "louisscruz",
"headCommitOid": "6f751ead182afb2a85e154775d3d685f96147034",
"mergeBaseOid": "6e45579c18ddc43972e1ee0a1a906b8b7dccf1e6",
"lastPushDate": "2024-07-04T01:46:52.000Z",
"lastActivityDate": "2024-08-06T19:48:18.000Z",
"hasMergeConflict": false,
"isFirstContribution": true,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "react",
"kind": "edit",
"files": [
{
"path": "types/react/global.d.ts",
"kind": "definition"
},
{
"path": "types/react/index.d.ts",
"kind": "definition"
},
{
"path": "types/react/test/elementAttributes.tsx",
"kind": "test"
}
],
"owners": [
"johnnyreilly",
"bbenezech",
"pzavolinsky",
"ericanderson",
"DovydasNavickas",
"theruther4d",
"guilhermehubner",
"ferdaber",
"jrakotoharisoa",
"pascaloliv",
"hotell",
"franklixuefei",
"Jessidhia",
"saranshkataria",
"lukyth",
"eps1lon",
"zieka",
"dancerphil",
"dimitropoulos",
"disjukr",
"vhfmag",
"hellatan",
"priyanshurav",
"Semigradsky",
"mattpocock"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "changereq",
"reviewer": "sheetalkamat",
"date": "2024-08-06T19:48:18.000Z"
}
],
"mainBotCommentID": 2212032563,
"ciResult": "pass"
}
🔔 @johnnyreilly @bbenezech @pzavolinsky @ericanderson @DovydasNavickas @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @hotell @franklixuefei @Jessidhia @saranshkataria @lukyth @eps1lon @zieka @dancerphil @dimitropoulos @disjukr @vhfmag @hellatan @priyanshurav @Semigradsky @mattpocock — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.
Is there a spec for MathML? For HTML and SVG we follow the spec instead of what browsers implement and let users augment their types with non-standard elements/attributes.
The added code needs to be ported to types/react/ts5.0 as well.
That makes sense. Yes, there are specifications. I added some more context on that in the PR description.
It ends up being that the Math WG is quite a bit older than I'd previously realized.... apparently, the group has been in operation for ~26 years! MDN describes the MathML standardization history as "erratic", and the Math WG describes MathML Core as "a subset that can be reliably displayed in web browsers". It seems like the MathML Core specification is perhaps the thing to support instead of MathML (v3, from 2014). That seems to also be the route Chrome has taken.
For now, I'll move forward with adjusting the PR to support the MathML Core specification. I'll take a pass in the next few days.
Re-ping @johnnyreilly, @bbenezech, @pzavolinsky, @ericanderson, @DovydasNavickas, @theruther4d, @guilhermehubner, @ferdaber, @jrakotoharisoa, @pascaloliv, @hotell, @franklixuefei, @Jessidhia, @saranshkataria, @lukyth, @eps1lon, @zieka, @dancerphil, @dimitropoulos, @disjukr, @vhfmag, @hellatan, @priyanshurav, @Semigradsky, @mattpocock:
This PR has been out for over a week, yet I haven't seen any reviews.
Could someone please give it some attention? Thanks!
Sorry for delay here, planning to take a look this week (and push up some changes).
It has been more than two weeks and this PR still has no reviews.
I'll bump it to the DT maintainer queue. Thank you for your patience, @louisscruz.
(Ping @johnnyreilly, @bbenezech, @pzavolinsky, @ericanderson, @DovydasNavickas, @theruther4d, @guilhermehubner, @ferdaber, @jrakotoharisoa, @pascaloliv, @hotell, @franklixuefei, @Jessidhia, @saranshkataria, @lukyth, @eps1lon, @zieka, @dancerphil, @dimitropoulos, @disjukr, @vhfmag, @hellatan, @priyanshurav, @Semigradsky, @mattpocock.)
@louisscruz One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!
@louisscruz I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Sep 5th (in a week) if the issues aren't addressed.
@louisscruz To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!
Ah, I'd forgotten about this PR! I'm planning to continue this proposed addition over in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/71187