console icon indicating copy to clipboard operation
console copied to clipboard

Add algorithm for console table

Open GasimGasimzada opened this issue 1 year ago • 12 comments

Define algorithm for console.table

  • Define the data structure that is passed to the Formatter
  • Create algorithm that creates the final structure based on tabularData and properties arguments

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

  • [ ] At least two implementers are interested (and none opposed):
  • [ ] Tests are written and can be reviewed and commented upon at:
  • [x] Implementation bugs are filed: Not a bug
  • [x] MDN issue is filed: Documentation is already in-place -- https://developer.mozilla.org/en-US/docs/Web/API/console/table_static
  • [ ] The top of this comment includes a clear commit message to use.

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


Preview | Diff

GasimGasimzada avatar Aug 11 '24 10:08 GasimGasimzada

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

zcorpan avatar Aug 15 '24 14:08 zcorpan

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.

GasimGasimzada avatar Aug 15 '24 16:08 GasimGasimzada

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

nchevobbe avatar Aug 15 '24 20:08 nchevobbe

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.

GasimGasimzada avatar Aug 15 '24 22:08 GasimGasimzada

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

AtkinsSJ avatar Aug 22 '24 08:08 AtkinsSJ

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

zcorpan avatar Aug 22 '24 11:08 zcorpan

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?

bmeurer avatar Aug 22 '24 13:08 bmeurer

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.

zcorpan avatar Aug 22 '24 14:08 zcorpan

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.

GasimGasimzada avatar Aug 22 '24 15:08 GasimGasimzada

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:

  • Express a more clear type than any for tabularData. E.g. maybe (sequence<any> or record<DOMString, any>). Note that this can throw during conversion from JS to Web IDL, e.g. if [Symbol.iterator] is a throwing getter, or if there are in general any throwing getters as keys.
  • Use raw ECMAScript spec operations to manipulate tabularData. This allows you full control over the handling of exceptional cases.

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

domenic avatar Aug 23 '24 04:08 domenic

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.

bmeurer avatar Aug 23 '24 06:08 bmeurer

@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);

zcorpan avatar Aug 23 '24 11:08 zcorpan