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

Enable Weak References by default #3384

Closed
luveti opened this issue Apr 12, 2023 · 6 comments
Closed

Enable Weak References by default #3384

luveti opened this issue Apr 12, 2023 · 6 comments
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever) enhancement

Comments

@luveti
Copy link

luveti commented Apr 12, 2023

Motivation

WeakRef is now supported by all mainstream browsers: https://caniuse.com/mdn-javascript_builtins_weakref

Proposed Solution

Remove WASM_BINDGEN_WEAKREF and set the default value to true.

Alternatives

Leak memory forever.. :)

Additional Context

N/A

@daxpedda
Copy link
Collaborator

Both reference types and multi-value are finished proposals and stable on all browsers for a long time now.

Multi-value is a bit more involved because it requires compiler support, which is currently broken anyway, but at least reference types can be enabled by default next to weak references too.

@daxpedda daxpedda pinned this issue May 12, 2023
@daxpedda daxpedda added the breaking-change Tracking breaking changes for the next major version bump (if ever) label May 25, 2023
@daxpedda daxpedda unpinned this issue May 25, 2023
@daxpedda
Copy link
Collaborator

Apparently there are runtimes, e.g. serverless, that don't support weak references. So I'm not sure if we can enable this by default without a breaking change.

@pheki
Copy link

pheki commented Sep 15, 2023

Could we use feature detection for WeakRef? Then we would avoid breaking old browsers and serverless while making it available to most browsers

@daxpedda
Copy link
Collaborator

daxpedda commented Sep 15, 2023

I never looked into what wasm-bindgen-cli does here exactly, but that's probably the best solution here.
Happy to review a PR for that.

@luxalpa
Copy link

luxalpa commented May 8, 2024

It seems this got fixed in 0.2.91? Can this be confirmed?
#3812
#3822

@Liamolucko
Copy link
Collaborator

It seems this got fixed in 0.2.91? Can this be confirmed?

#3812

#3822

Yes, except that it has a pretty severe flaw right now: #3854.

#3854 is the correct place to track that though, not here, so I'll close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever) enhancement
Projects
None yet
Development

No branches or pull requests

5 participants