Skip to content
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

Diff against modern typescript definitions #486

Open
oyvindberg opened this issue Aug 13, 2021 · 11 comments
Open

Diff against modern typescript definitions #486

oyvindberg opened this issue Aug 13, 2021 · 11 comments
Labels

Comments

@oyvindberg
Copy link

AFAIU scala-js-dom was originally generated based on some ancient version of typescript, in the 1.x days and manually patched since. In the meantime those definitions have seen considerable expansion and refactoring, as well as being based on the standard web idl.

I have almost all the pieces to diff scala-js-dom with those typescript definitions to generate a report with all missing/different methods, fields and types.

I volunteer to do that next week, so we can get an idea of how much surface we're missing.

@oyvindberg
Copy link
Author

Also there is all this code: https://github.com/ScalablyTyped/Distribution/tree/master/s/std/src/main/scala/typings/std

everything is there. It has a few scalablytypedisms we may want to change/delete, but it should be a treasure trove for copy/pasting into this repo.

@armanbilge
Copy link
Member

@oyvindberg see my comment #481 (comment). At the moment, I am feeling confused why that doesn't just become scala-js-dom.

@oyvindberg
Copy link
Author

oyvindberg commented Aug 13, 2021

@armanbilge I see a lot of value in having a stable and conservative version of the DOM, scala-js-dom has seen very little changes over the years and it has absolutely not been a source of problems.

Autogenerated is better for many other things, like if you need an up to date and complete version, possibly tailored to your environment/preference by rewriting all null to Option, wrapping all effects in IO or things like that. The problem with autogenerated things like the code I linked is that it's volatile, small changes in the original version may easily cause binary-incompatible changes in the translated version.

So I see two paths going forward regarding this:

  1. take a snapshot of std, customize it (perhaphs heavily) and consider it frozen for another many years
  2. take the existing scala-js-dom and selectively apply changes from the generated code to cover the most relevant cases

2 sounds like less pain to me.

edit: less pain for downstream in particular.

@armanbilge
Copy link
Member

armanbilge commented Aug 13, 2021

Thank you for this explanation, very helpful :) Regarding 1 vs 2, idk between type aliases and @japgolly magical scalafixes 1 could be less painful that it might seem (but still more than 2 for sure).

The problem with autogenerated things like the code I linked is that it's volatile, small changes in the original version

The original version here being lib.dom.d.ts correct? Specifically, this is what we want to freeze, or if perhaps we are clever, we maintain our own fork and merge in changes from upstream so long as they are binary compatible. The idea being to offload as much of the maintenance burden as possible onto the typescript folks.

In fact, as of #471 we are already effectively using a document like that to keep track of the API changes. Is it just me, or does this seem like a lot of duplicated effort?

@armanbilge
Copy link
Member

@oyvindberg do you still have time to look into this? No worries if not! :)

@oyvindberg
Copy link
Author

No, sorry. I'll contribute this sometime later :)

@armanbilge armanbilge reopened this Sep 4, 2021
@armanbilge
Copy link
Member

I'll leave this open, still a good thing to do!

@armanbilge
Copy link
Member

armanbilge commented Sep 6, 2021

Now that we are de-namespacing everything in #545, I think should be much easier?

Actually, more than a diff of scala-js-dom against the typescript definitions, I'm curious about a diff against ST's generated source. I am very curious how far away we could be from just "flipping the switch" on automation #487 from a source-compatibility perspective.

Btw just jotting this down as it popped into my head, no expectation that anyone does this and certainly not anytime soon :)

@benhutchison
Copy link
Contributor

So I see two paths going forward regarding this:

  1. take a snapshot of std, customize it (perhaphs heavily) and consider it frozen for another many years
  2. take the existing scala-js-dom and selectively apply changes from the generated code to cover the most relevant cases

2 sounds like less pain to me.

Context: It's a quiet Sunday morning and I'm circling back to dust off & modernize some 7yo code of mine that used Pointer Events .. and that made me ask whether Pointer Events had made it into the facade.

Last time I raised it in 2015, they hadn't due to immaturity, but browser support has flipped Red to Green since then.

As always, @oyvindberg is all over it, with PointerEvent avail in Scalably Typed, derived from TypeScript.

After some issue reading to understand state of play, I'm left weighing up two options:

  • Propose a patch on scala-js-dom that copies over PointerEvent & friends.
  • Switch my codebase to depend on Scalably Typed's machine generator.

Both options have pros & cons. A patch to scala-js-dom might allow using nice Ints over raw Doubles. And scala-js-dom is published to maven, so the usage story is more "normal" than code generation.

OTOH, hand-maintained scala-js-dom feels a little obsolete next to the efficiency of auto-generation from TS interfaces.

Overall Im tending towards patch because it's less change & "perfect is enemy of good" etc..

Also, I find generated ScalablyTyped facades often harder to make sense of than I naively expect, eg Im not sure what all these refs to pointerenter in s/std/ are and if I need to care. So I still see value in a smaller, curated interface from a usability POV.

I mention all this here because it hopefully helps inform the future plan and provides one concrete example of a use case.

@armanbilge
Copy link
Member

armanbilge commented Feb 6, 2022

Thanks @benhutchison, that was very helpful commentary. I linked to it from #487 since its relevant to that discussion.

In the meantime, PRs for PointerEvent and friends are welcome :)

Edit: btw, another change since 2015 is we'll now acccept facades for any specced API, regardless of its support in browsers. See: https://github.com/scala-js/scala-js-dom/blob/main/CONTRIBUTING.md#facades

@benhutchison
Copy link
Contributor

I'm not sure how I missed this, but anyway pleased to report that this facade already has PointerEvents.. since 2018! 😝

🙏 to @busti & @sjrd for #317

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants