search

whatwg / console

Console Standard
https://console.spec.whatwg.org/
Other
274 stars 70 forks source link

Add algorithm for console table #237

Open GasimGasimzada opened 3 months ago

GasimGasimzada commented 3 months ago

Define algorithm for console.table

Description

I have tried to figure out and define the algorithm for console.table based on how Chromium and Firefox do it. This is my first PR; so, please let me know of any concerns and comments.

Checklist

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

zcorpan commented 3 months ago

Does the algorithm match Chrome or Firefox, or neither? cc @nchevobbe

GasimGasimzada commented 3 months ago

Does the algorithm match Chrome or Firefox, or neither? cc @nchevobbe

@zcorpan It matches both Chromium and Firefox. I did not see any difference between them for the cases that I tested.

nchevobbe commented 3 months ago

Does the algorithm match Chrome or Firefox, or neither? cc @nchevobbe

@zcorpan It matches both Chromium and Firefox. I did not see any difference between them for the cases that I tested.

There's one difference I'm aware of (see https://bugzilla.mozilla.org/show_bug.cgi?id=1744441)

With 

var client=[];
client["a"]=[1];
client["b"]=[2];
console.table(client);

Firefox and Safari don't print a table, Chrome and Node do. Given 

3. If |tabularData| is a [=/list=], then:
  1. Let |indices| be [=list/get the indices=] of |tabularData|

and https://infra.spec.whatwg.org/#lists :

To get the indices of a list, return the range from 0 to the list’s size, exclusive.

https://infra.spec.whatwg.org/#the-exclusive-range

The range n to m, exclusive, creates a new ordered set containing all of the integers from n up to and including m − 1 in consecutively increasing order, as long as m is greater than n. If m equals n, then it creates an empty ordered set.

I guess non integer indices shouldn't be included, which is what Safari and Firefox are doing

GasimGasimzada commented 3 months ago

I guess non integer indices shouldn't be included, which is what Safari and Firefox are doing

I did not check the use case of setting non integer indices to an array. Personally, my preference is that arrays only handle integer indices which is aligned with what Safari and Firefox (and LadyBird once my PR is ready to be merged :D) are doing. But it is just a personal preference and I do not have a strong opinion either way. Let me know how you want to proceed with this.

AtkinsSJ commented 3 months ago

This is now live in Ladybird (https://github.com/LadybirdBrowser/ladybird/pull/1027) if anyone wants to experiment with it.

zcorpan commented 3 months ago

@bmeurer can this be changed in Chromium/V8? See https://github.com/whatwg/console/pull/237#issuecomment-2292207723 above.

bmeurer commented 3 months ago

can this be changed in Chromium/V8? See #237 (comment) above.

We can definitely update Chromium accordingly, but I wonder if that wouldn't be considered a regression. Being able to print objects (with non-indexed properties) as tables could be useful for some users. Is there some data that we can base the decision to stick to just indexed properties on?

zcorpan commented 3 months ago

I think the idea is that a JS object (that is not an array) would still work (step 4).

Hmm, the spec algorithm here operates on Infra value types (list vs map), but the IDL type for JS arrays and JS objects are both "object" if I understand https://webidl.spec.whatwg.org/#js-any correctly. ( https://webidl.spec.whatwg.org/#js-union can map to an IDL sequence, step 11.1.)

I'm not sure what the JS - IDL - Infra mapping is for an IDL any type... If a JS array maps to IDL object and then Infra map, then https://github.com/whatwg/console/pull/237/files#diff-5e793325cd2bfc452e268a4aa2f02b4024dd9584bd1db3c2595f61f1ecf7b985R129 is always false.

GasimGasimzada commented 3 months ago

I'm not sure what the JS - IDL - Infra mapping is for an IDL any type... If a JS array maps to IDL object and then Infra map, then https://github.com/whatwg/console/pull/237/files#diff-5e793325cd2bfc452e268a4aa2f02b4024dd9584bd1db3c2595f61f1ecf7b985R129 is always false.

@zcorpan I am new to writing a spec and this is something I was not aware of. Just want to understand this for this use-case and as a future reference as well:

  1. Since JS array is a JS object underneath, should I treat both array and object as a map?
  2. If (1) is true, if I want to only iterate over indexed values of a JS object, how would I express this? I am asking this because there is no method in map for getting indices of an item while there is a method on it in the list.
domenic commented 3 months ago

The spec written here is not clear enough one way or another.

tabularData is specified as any. any is a Web IDL type representing an opaque handle to a JavaScript value, so it can never be an Infra list or map. So right now the spec never enters steps 3, 4, or 5 and always falls back to step 6.

You need to either:

The former would be better but I'm unsure how compatible it is with current implementations.

bmeurer commented 3 months ago

I think the idea is that a JS object (that is not an array) would still work (step 4).

Ah, I misread the example. If it's only about excluding non-indexed properties for arrays (or array-like objects), we can definitely do that in Chromium.

zcorpan commented 3 months ago

@domenic I think the second option is closer to what browsers do today. Gecko uses any. This doesn't throw in Safari/Chrome/Firefox:

let o = {}; Object.defineProperty(o, "x", { get() { throw "y" } }); console.table(o);