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

Remove ServiceWorkerContainer.onerror #380

Merged
merged 3 commits into from
Aug 25, 2021

Conversation

unarist
Copy link
Contributor

@unarist unarist commented Sep 9, 2019

It looks like not documented in the spec nor implemented in browsers.
see w3c/ServiceWorker#912

The current spec has messageerror event instead, but I'm omitting it because it is not described at MDN nor implemented Chrome for now. Tell me if I should include it in this PR.

@armanbilge
Copy link
Member

@unarist apologies that we are just getting to this, thank you for your contribution!! Have there been any relevant updates to the spec since your PR?

@armanbilge armanbilge changed the base branch from master to series/1.x August 10, 2021 01:37
@japgolly japgolly marked this pull request as draft August 12, 2021 23:54
@japgolly japgolly modified the milestones: v1.2.0, v2.0.0 Aug 13, 2021
@unarist
Copy link
Contributor Author

unarist commented Aug 24, 2021

Have there been any relevant updates to the spec since your PR?

The spec seems to not changed about this.

MDN docs had been updated to deprecate ServiceWorkerContainer.onerror.

For ServiceWorkerContainer.onmessageerror, MDN still not documented this property but Chrome had been implemented this (before my PR??).

@armanbilge
Copy link
Member

@unarist thanks for getting back on this! onerror seems long gone, let's proceed with your PR for 1.2.0.

@armanbilge armanbilge modified the milestones: v2.0.0, v1.2.0 Aug 24, 2021
@armanbilge armanbilge marked this pull request as ready for review August 24, 2021 11:23
@armanbilge armanbilge requested a review from japgolly August 24, 2021 11:23
@japgolly japgolly modified the milestones: v1.2.0, v2.0.0 Aug 25, 2021
@japgolly japgolly self-assigned this Aug 25, 2021
@japgolly
Copy link
Contributor

japgolly commented Aug 25, 2021

Ok @unarist & @armanbilge , in order to preserve backwards-binary-compatibility, I've restored the field but marked it as deprecated. We'll remove all the deprecated stuff on the 2.x branch (or not)

@armanbilge
Copy link
Member

Wait, why is this binary incompatible?

@armanbilge
Copy link
Member

This is a facade so it falls under the exceptions: #461 (comment)

@japgolly
Copy link
Contributor

ARGH! Thanks @armanbilge , I forgot this case is an SJS bincomat exception. D'oh. I'll revert...

@armanbilge
Copy link
Member

I wonder if we could use your scalafix to generate list of MiMa exceptions, so we can just run MiMa. One day!

@japgolly
Copy link
Contributor

Actually yeah, that's a really good idea. I should be able to implement that reasonably quickly. I'll see if I can smash it out today...

@armanbilge
Copy link
Member

@japgolly You rock!! Your scalafix wizardry has already opened my eyes 🤩 Plus I think this could be super-helpful for a future auto-generation scheme.

@japgolly japgolly merged commit c4cb4c1 into scala-js:series/1.x Aug 25, 2021
@japgolly
Copy link
Contributor

Boom. Merged. Thanks for the PR (and your incredible patience) @unarist , and thanks for you attentive reviewing. :)

@armanbilge
Copy link
Member

That was our last PR! How do you feel about cutting a release @japgolly? The remaining issues look like infrastructure/internal stuff mostly: https://github.com/scala-js/scala-js-dom/milestone/1

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.

3 participants