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

Misc: prepare the release #1827

Merged
merged 2 commits into from
Feb 7, 2025
Merged

Misc: prepare the release #1827

merged 2 commits into from
Feb 7, 2025

Conversation

hhugo
Copy link
Member

@hhugo hhugo commented Feb 4, 2025

No description provided.

@hhugo hhugo mentioned this pull request Feb 4, 2025
@vouillon
Copy link
Member

vouillon commented Feb 4, 2025

@hhugo There are some concerns from Jane Street that #1769 might be too aggressive. In particular, they are relying on the fact that the method localStorage of window has type storage t optdef readonly_prop to access safely the local storage when it is available, without crashing in other contexts (node / service workers). Also, Chrome is sending fake mousedown events without a key nor a code property.

I think it makes sense to revert the changes for the localStorage and sessionStorage. I'm not so sure regarding key events. @rickyvetter @TyOverby What do you think?

@TyOverby
Copy link
Collaborator

TyOverby commented Feb 4, 2025

I think that something needs to be done about the events handler types.

The “correct” change would be to remove the subtypes from all event handlers, requiring the user to try to upcast an Event to Keyboard event.

I think it would be easier to keep all of the fields as optdef, allowing people to continue “duck typing” with the fields.

But the current state is not great for us, we have a lot of applications that handle these fields not being present, and if we were to remove the optdef checks, they’d just crash instead

* Revert some of #1769 for compatibility

* Fix CI
@hhugo hhugo merged commit b750638 into master Feb 7, 2025
29 of 30 checks passed
@hhugo hhugo deleted the release-6.0 branch February 7, 2025 10:36
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