-
Notifications
You must be signed in to change notification settings - Fork 75
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
Manual patches pending MDN data updates #35
Comments
Haha, no problem. I was just about to start doing it my self when your PR showed up. So I was kind of expecting this to happen anyway. :)
Sounds quite reasonable. It could also be missing keywords or data types to existing properties and that could be troubling in some cases when merging two interfaces that share some properties. I would rather not use
More advanced than this isn't needed because they will resolve to string anyway. With this solution we could also output some build information in the terminal when the patch can be safely removed. |
Agreed, we shouldn't narrow the range of supported TS versions unnecessarily.
I hadn't thought of that, that's a great point! |
I've started a WIP pull request for this. I've a question there:
Should we introduce some kind of prerelease for this? That the stable contains typings from MDN data only (except for the SVG for now) and the prereleases contains typings from MDN data and patches? |
So then there would be a prerelease branch that had to continually merge from In the unlikely event that such a thing happened though, I'd advocate first marking the property I think putting these things on a prerelase would also be a major headache for downstream users who want these patch properties (which is, I think, everyone?). Consider: most users aren't depending directly on So my feeling is there should just be a single stream of versions: the latest combination of autogenerated and patched properties. Over time, some properties will be marked deprecated and then maybe removed, and that's fine. |
Yeah, you're probably right. The idea just popped up in my head with no major reflection.
I though so too. But then @wbamberg posted this comment to one of my PRs. It would be really interesting to hear what the policy is. Because we both have cases with old specs and some really old ones too. Like vendor properties that doesn't even have any official spec but works great in some versions. It's just that I don't know 100% how to handle issues like #41 and #39 at the moment and how MDN would react on adding them. Adding properties as a patch and then remove them because MDN rejected it doesn't sound trivial either. It will become very frustrating for all of our end-users. |
I think we should aim to only patch things that are officially accepted parts of the spec, which MDN data is just temporarily missing for whatever reason. As noted in that comment, |
Particularly now that React is using CSSType's
Properties
without a string index fallback, we're discovering more cases where the MDN data is missing properties and it's more pressing to have the typings be complete (sorry if I jumped the gun in creating this pressure!). It might be nice to have a release valve in the form of an interface of manual patches that we mix in toProperties
, so we don't have to wait on MDN to merge PRs in order to be responsive to these missing properties. Then once the MDN PRs do get merged and the properties can be generated, we can delete them from the manual patches interface. This is essentially the approach taken with the SVG properties; would it make sense to generalize it?Also if we take this approach I'm happy to volunteer to start creating PRs to patch the known missing properties!
The text was updated successfully, but these errors were encountered: