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

Implement js.lib.WeakRef #10488

Merged

Conversation

dimensionscape
Copy link
Contributor

@dimensionscape dimensionscape commented Nov 17, 2021

WeakRef was introduced in ES2021 and is now supported on every major browser. I think it's time for Haxe to include it as well.

WeakRef was introduced in ES6 and is now supported on every major browser. I think it's time for Haxe to include it as well.
@haxiomic
Copy link
Member

haxiomic commented Nov 17, 2021

Minor tweak, maybe it's worth adding the constraint {}: class WeakRef<T: {}> as you can only have weak refs to objects and it's a runtime error to ref primitives like Int or String

WeakRef was introduced in ES2021 so it's quite new but still widely supported (~87% https://caniuse.com/?search=weakref) so I agree it's fair for it to be in js.lib

@dimensionscape
Copy link
Contributor Author

dimensionscape commented Nov 17, 2021

Minor tweak, maybe it's worth adding the constraint {}: class WeakRef<T: {}> as you can only have weak refs to objects and it's a runtime error to ref primitives like Int or String

Absolutely correct. I actually didn't notice that I forgot to add the constraint when applying the Type param changes to my original commit. Thanks!

WeakRef was introduced in ES2021 so it's quite new but still widely supported (~87% https://caniuse.com/?search=weakref) so I agree it's fair for it to be in js.lib

Correct again. My apologies - I hadn't even had my morning coffee yet at the time of writing. Perhaps I was conflating with WeakMaps, but I'm not sure why I said ES6.

As Haxiomic pointed out, I forgot the Type constraint required to prevent primitives from being used which would result in a runtime error.
@RealyUniqueName RealyUniqueName merged commit f69ad40 into HaxeFoundation:development Nov 18, 2021
@skial skial mentioned this pull request Nov 18, 2021
1 task
@haxiomic
Copy link
Member

haxiomic commented Nov 18, 2021

All good :], thank you for the contribution <3

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