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

WebAssembly support not easy to find #16952

Closed
RobertoMalatesta opened this issue Nov 11, 2017 · 38 comments
Closed

WebAssembly support not easy to find #16952

RobertoMalatesta opened this issue Nov 11, 2017 · 38 comments
Labels
util Issues and PRs related to the built-in util module.

Comments

@RobertoMalatesta
Copy link

RobertoMalatesta commented Nov 11, 2017

  • Version: 9.0.0, 9.0.1, 8.3.0
  • Platform: Kubuntu 14.04, CentOS Linux release 7.2.1511 (Core)
  • Subsystem: (node versions managed by n package)
node -v
v9.0.0
node
> WebAssembly
{}

node --expose-wasm 
> WebAssembly
{}

=========
[as root] n latest
=========

node -v
v9.1.0

node
> WebAssembly
{}
> 

node --expose-wasm 
> WebAssembly
{}

========
on CentOS:
========

 node -v
v8.3.0
[---]$  node
> WebAssembly
{}
> [---]$  node --expose-wasm
> WebAssembly
{}


Thanks for your time,

--Rob

@addaleax
Copy link
Member

Hi, thanks for the issue!

Maybe I’m misunderstanding, but WebAssembly support is there in Node 8 & 9, with or without the flag.

All the functions are there:

$ node
> process.version
'v9.0.0'
> WebAssembly.compile
[Function: compile]

So I guess the real issue here is that WebAssembly is displayed as {} in the console?

@RobertoMalatesta
Copy link
Author

RobertoMalatesta commented Nov 11, 2017

Yep @addaleax .

WebAssembly (formerly wasm) should show its available API, IMHO.

Just like i.e. console:


console
Console {
  log: [Function: bound consoleCall],
  info: [Function: bound consoleCall],
  warn: [Function: bound consoleCall],
  error: [Function: bound consoleCall],
  dir: [Function: bound consoleCall],
  time: [Function: bound consoleCall],
  timeEnd: [Function: bound consoleCall],
  trace: [Function: bound consoleCall],
  assert: [Function: bound consoleCall],
  clear: [Function: bound consoleCall],
  count: [Function: bound consoleCall],
  countReset: [Function: bound countReset],
  Console: [Function: Console],
  debug: [Function: debug],
  dirxml: [Function: dirxml],
  table: [Function: table],
  group: [Function: group],
  groupCollapsed: [Function: groupCollapsed],
  groupEnd: [Function: groupEnd],
  markTimeline: [Function: markTimeline],
  profile: [Function: profile],
  profileEnd: [Function: profileEnd],
  timeline: [Function: timeline],
  timelineEnd: [Function: timelineEnd],
  timeStamp: [Function: timeStamp],
  [Symbol(counts)]: Map {} }

--R

@addaleax addaleax added the util Issues and PRs related to the built-in util module. label Nov 11, 2017
@addaleax
Copy link
Member

addaleax commented Nov 11, 2017

I think a good solution would be to solve this by taking @@toStringTag into account as a fallback when computing the constructor name in util.inspect. That’s kind of easy to do, so I’m labeling this as a good first issue if anybody wants to grab it.

@addaleax addaleax added good first issue Issues that are suitable for first-time contributors. mentor-available labels Nov 11, 2017
@apapirovski
Copy link
Member

apapirovski commented Nov 11, 2017

Is this actually incorrect? It matches what all the browsers do... Chrome is the only one that shows the internals but they're faded like private properties are usually.

@addaleax
Copy link
Member

@apapirovski I think not displaying the methods is correct, yes. But I think there really should be output that distinguishes WebAssembly from a simple {}.

@apapirovski
Copy link
Member

@addaleax but it matches browser consoles and that's generally been at least partially our goal.

@RobertoMalatesta
Copy link
Author

I feel it as a lack of interactive documentation.

@addaleax
Copy link
Member

@apapirovski I think the goal of util.inspect is to be as helpful as possible … if we can be more helpful than browsers I’ll take it. (And yes, helpful is very subjective. 😄)

@RobertoMalatesta
Copy link
Author

RobertoMalatesta commented Nov 11, 2017

WebAssembly examples online still use wasm and instantiateModule.
Having a JS object exposing its public methods is a good practice imho.

--R

@addaleax
Copy link
Member

@RobertoMalatesta To be clear, what I’m suggesting is to switch to something like WebAssembly {} as the displayed output, because we generally stick with what the object says – if its properties are not enumerable, then we won’t show them in the console either by default.

The methods are all there, it’s really just a question of how to display the object in question.

@RobertoMalatesta
Copy link
Author

Fine for me if there's an easy way to follow its API development (not only for me but for anyone interested).
util.inspect returns {} btw
So, where a user should see the info on WebAssembly module?
(it's not in oficial doc on main site)

Thanks for your work, (just trying to be of help).

--Roberto

@apapirovski
Copy link
Member

apapirovski commented Nov 11, 2017

I'm still not convinced we should be doing anything differently.

The WebAssembly object is the initial value of the WebAssembly property of the global object. Like the Math and JSON objects, the WebAssembly object is a plain JS object (not a constructor or function) that acts like a namespace and has the following properties:

https://github.com/WebAssembly/design/blob/master/JS.md

It's a plain JS object with non-enumerable properties. It's not a class, constructor, whatever. Naming it as WebAssembly {} seems misleading.

@apapirovski
Copy link
Member

> Math
{}
> JSON
{}

@RobertoMalatesta
Copy link
Author

RobertoMalatesta commented Nov 11, 2017

@apapirovski ok. no matter where you can put those infos, but either they're secret or they have to be documented.
Else, it's bad documentation and user experience for the devs.

Math and JSON are waay documented :P.

@apapirovski
Copy link
Member

@RobertoMalatesta But Node isn't even the source of WebAssembly, V8 is and it's part of the JS documentation out there. It doesn't make sense for Node to document WebAssembly.

https://developer.mozilla.org/en-US/docs/WebAssembly

@apapirovski
Copy link
Member

We don't document Math and JSON... others do and they do the same for WebAssembly.

@addaleax
Copy link
Member

addaleax commented Nov 11, 2017

@apapirovski Right, I would really like it for Math and JSON we could also use @@toStringTag… It’s not like we’d be special-casing anything.

@apapirovski
Copy link
Member

@addaleax Fair enough. 👍

@RobertoMalatesta
Copy link
Author

We don't document Math and JSON... others do and they do the same for WebAssembly.

Is there a performance reason not doing that?
If so, OK.

@apapirovski
Copy link
Member

It's about conformance to the spec. The properties are not enumerable and are not meant to be shown. The proposal by @addaleax is to have it show Math {}, JSON {} and WebAssembly {} instead of just {}.

@RobertoMalatesta
Copy link
Author

node-v7.2.1-linux-x64/bin/node --expose-wasm
> Wasm
{ verifyModule: [Function],
  verifyFunction: [Function],
  instantiateModule: [Function],
  experimentalVersion: 11 }
>

@apapirovski
Copy link
Member

apapirovski commented Nov 11, 2017

Yes, the version of wasm in V8 in that release had enumerable properties. If you have a problem with this behaviour then take it up with the spec writers or V8. Node is not the issue here.

@RobertoMalatesta
Copy link
Author

OK. Thanks for your help, and time on nodejs
--Roberto.

@apapirovski
Copy link
Member

I'm keeping this open as we still want to implement the @@toStringTag bit.

@apapirovski apapirovski reopened this Nov 11, 2017
@RobertoMalatesta RobertoMalatesta changed the title WebAssembly support not found WebAssembly support not easy to find Nov 11, 2017
@devsnek
Copy link
Member

devsnek commented Nov 11, 2017

i'll open a pr in a few for this, although i think its fair to mention that even with @@toStringTag WebAssembly will remain a console mystery:

@addaleax
Copy link
Member

@devsnek Yea, I’m not super happy about displaying WebAssembly {} either, but I agree with @apapirovski in that we should avoid creating special cases when possible…

@apapirovski
Copy link
Member

apapirovski commented Nov 11, 2017

I mean, that's what util.inspect(WebAssembly, true, 0) is for. It would be a bit weird to make an exception for JSON, Math & WebAssembly.

@devsnek
Copy link
Member

devsnek commented Nov 11, 2017

i think we're all in agreement at this point. 👀

@apapirovski
Copy link
Member

Well, I think there's a merit here to saying that showHidden should be on by default in REPL, then this would not be an issue. It would make our logging more similar to what the Chrome console does but less similar to Safari, Firefox & Edge.

@devsnek
Copy link
Member

devsnek commented Nov 11, 2017

+1 to using showHidden in REPL

@devsnek
Copy link
Member

devsnek commented Nov 11, 2017

@apapirovski i'm not so sure about this behavior, my only thought is to apply separate depth limits to enumerable and non-enumerable properties on util.inspect

@mscdex
Copy link
Contributor

mscdex commented Nov 11, 2017

The proposal by @addaleax is to have it show Math {}, JSON {} and WebAssembly {} instead of just {}.

-1 The name before the braces should be a class/constructor name, not a namespace, for consistency.

@addaleax addaleax removed good first issue Issues that are suitable for first-time contributors. mentor-available labels Nov 12, 2017
@joyeecheung
Copy link
Member

joyeecheung commented Nov 12, 2017

@mscdex It's not a name space, but the @@toStringTag name. IMO I see no harm in displaying those for a better description when constructor.name is too vague.

@mscdex
Copy link
Contributor

mscdex commented Nov 12, 2017

@joyeecheung In these cases, the @@toStringTag is just returning the name of the global variable (e.g. Math, JSON, etc.), so I would call that a namespace, especially since they are not constructors (or functions at all).

@joyeecheung
Copy link
Member

joyeecheung commented Nov 12, 2017

@mscdex But this does not have to be about those built-in obejects. Considering this:

var foo = {};
Object.defineProperty(foo, Symbol.toStringTag, { value: 'Foo', enumerable: false })

IMO in this case it's reasonable to display foo as Foo {}. constructor.name is just one way to get the class/type name, but not every object is supposed to be constructed by a constructor or by calling new.

@mscdex
Copy link
Contributor

mscdex commented Nov 12, 2017

@joyeecheung Using the same notation as constructor names is misleading and inconsistent.

@addaleax
Copy link
Member

@joyeecheung @mscdex It would be nice if you could chime in at what’s being discussed in #16956 (comment) as a solution

@targos
Copy link
Member

targos commented Jan 2, 2018

#16956 landed. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants