Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Potential web compatibility issue with exports of globals (forked from thread repo) #1

Closed
binji opened this issue Jan 16, 2018 · 11 comments

Comments

@binji
Copy link
Member

binji commented Jan 16, 2018

See WebAssembly/threads#73.

@binji
Copy link
Member Author

binji commented Jan 16, 2018

@lars-t-hansen
Copy link
Collaborator

lars-t-hansen commented Jan 26, 2018

cf WebAssembly/threads#73 (comment)

The experiment to export WebAssembly.Global instances (and allow them in imports) has landed in Nightly, we'll monitor the fallout. Maybe when we get close to the next beta date (mid March) we'll leave it on in the early betas to get more exposure.

@kripken
Copy link
Member

kripken commented Feb 7, 2018

This broke emscripten's dynamic linking tests for globals (which are exported constants). I was very puzzled by this as the exported globals end up printed correctly as integers, but their typeof and JSON.stringify are of empty objects.

Anyhow, I doubt anyone uses that dynamic linking support, and I can just fix it. I'm not sure how, though - the new thing returned doesn't have any enumerable properties, there seems to be no way to get the numeric value except to do '' + exported (as the string value - but not stringify - has the correct number)?

@domenic
Copy link
Member

domenic commented Feb 8, 2018

exported.valueOf() should work.

It may be worth adding a toJSON() as well if we anticipate serializing globals to JSON being common?

@lars-t-hansen
Copy link
Collaborator

lars-t-hansen commented Feb 8, 2018

WebAssembly.Global instances have a 'value' accessor. (Mutable globals also have a 'value' setter.)

js> var ins =
  new WebAssembly.Instance(
    new WebAssembly.Module(
      wasmTextToBinary(`(module (global (export "g") i32 (i32.const 37)))`)))
js> ins.exports.g.value
37

@kripken
Copy link
Member

kripken commented Feb 8, 2018

Interesting, it doesn't show up when enumerating or in JSON.stringify, so it's kind of "hidden" unless you know what to look for ;) Seems odd to me, but I'm not very knowledgeable about JS spec issues, probably there's a deep reason I don't know.

@lars-t-hansen
Copy link
Collaborator

It's normal that data properties are not enumerable (https://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects, the last few paragraphs of section 17). That it doesn't show up in JSON.stringify follows directly from that. As @domenic said, perhaps we want to add a JSON hook for that reason, as eg Date has (https://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects).

@littledan
Copy link
Contributor

@domenic

exported.valueOf() should work.

I agree that this is a good way to do the conversion to primitive, rather than Symbol.toPrimitive. Of course, not everything will end up casting to a primitive--this will only help so much. I think @binji's current draft spec does a good job here.

@kripken

Anyhow, I doubt anyone uses that dynamic linking support, and I can just fix it.

Why do you think this feature is unused?

@lars-t-hansen

It's normal that data properties are not enumerable

Everything in the WebAssembly JS API has switched from non-enumerable to enumerable in the new WebIDL formalization. I was considering calling this out in appendix, but @rossberg was encouraging this to be in a separate document, which I unfortunately have not yet prepared. I apologize for my delay in fixing up the jsapi.js tests to check for enumerability rather than non-enumerability.

@kripken
Copy link
Member

kripken commented Mar 2, 2018

@littledan

Why do you think this feature is unused?

No concrete data, but based on issues filed etc. my impression is that practically no one uses dynamic linking in emscripten.

@lars-t-hansen
Copy link
Collaborator

WebAssembly.Global has been in Firefox Nightly and early Betas for a while now, and apart from the one bug reported by @kripken above, I've seen no traffic to indicate that this breaks anything.

I tried searching for additional issues that have not come into bugzilla, but found nothing.

I think it's reasonable to assume that introducing WebAssembly.Global as a breakable change will not actually break anything.

@binji
Copy link
Member Author

binji commented Apr 27, 2018

OK, it seems like we're going to go forward with the (slightly) breaking change, so I'll close this issue.

@binji binji closed this as completed Apr 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants