lib: align console.table row to the left

[MrJithil]

Fixes: #50117

MDN: https://developer.mozilla.org/en-US/docs/Web/API/console/table

[Uzlopak]

I am fine with this, but I checked the "spec" and it says nothing regarding the output. Its even worse, LOL

See

https://console.spec.whatwg.org/#table

[MrJithil]

I am fine with this, but I checked the "spec" and it says nothing regarding the output. Its even worse, LOL

See

https://console.spec.whatwg.org/#table

Yeah. The WHATWG, is not suppose mention the standards in very detail. They have the links to MDN.

[Uzlopak]

Yes and the mdn does not state that the entries are left aligned but just contains a screenshot.

[meyfa]

What was the original reason for centering it in Node.js? Is there a git blame?

[marco-ippolito]

I have the feeling this might break some stuff based on snapshots/benchmarks

[benjamingr]

LGTM this aligns with browser behavior for a primarily browser-compat-reasons API.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/55236/

[Uzlopak]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/55265/

[nodejs-github-bot]

Landed in 14af167

[RafaelGSS]

It hasn't released yet. Thanks for catching up @marco-ippolito.

Update: Wait, if this is now following the SPEC I wonder if this should be considered semver-major.

[aduh95]

The spec doesn't say anything about alignment, technically Node.js was spec-compliant before and after this PR landed. What changes is now Node.js output is closer to what browsers DevTools output.

But anyway, I don't really how this change could break anyone, AFAICT the change is only visual.

[RafaelGSS]

AFAICT the change is only visual.

Output changes are considered semver-major. Marco tagged it because we've just mentioned it during the collab summit.

[marco-ippolito]

It hasn't released yet. Thanks for catching up @marco-ippolito.

Update: Wait, if this is now following the SPEC I wonder if this should be considered semver-major.

There is no SPEC, now it just has the same behavior of browser

[aduh95]

AFAICT the change is only visual.

Output changes are considered semver-major. Marco tagged it because we've just mentioned it during the collab summit.

I don’t think that’s true, for example changing an error message technically changes the output, yet we don’t consider it semver-major. Also the output of console.table is not observable from JavaScript without watching directly the stdout.

[RafaelGSS]

I don’t think that’s true, for example changing an error message technically changes the output, yet we don’t consider it semver-major

We do consider error message changes as semver-major or at least, we should.

[aduh95]

@RafaelGSS that's not what we say in our documentation:

node/doc/api/errors.md

Lines 285 to 289 in 7c1b1f4

	The `error.code` property is a string label that identifies the kind of error.
	`error.code` is the most stable way to identify an error. It will only change
	between major versions of Node.js. In contrast, `error.message` strings may
	change between any versions of Node.js. See [Node.js error codes][] for details
	about specific codes.

We also recommend testing only the error code, not the message:

node/doc/contributing/writing-tests.md

Lines 315 to 325 in 260092e

	In the case of internal errors, prefer checking only the `code` property:
	```js
	assert.throws(
	() => {
	throw new ERR_FS_FILE_TOO_LARGE(`${sizeKiB} Kb`);
	},
	{ code: 'ERR_FS_FILE_TOO_LARGE' },
	// Do not include message: /^File size ([0-9]+ Kb) is greater than 2 GiB$/
	);
	```

AFAICT, it's always been the case that we have been treating error message update as semver-patch, and recommend users to use the code property to detect errors.

Anyway, to come back to this PR, I really think this is not semver-major, but it's not a hill I'll die on.

[RafaelGSS]

Oh, I haven't seen or I don't remember these sections, thanks for pointing that out. If we have a doc or a precedence, I won't object to it. @marco-ippolito I will remove the label. In case you strong believe it should be semver-major, ping me.

[RafaelGSS]

FWIW a resource I found in a quick search #3374.

UPDATE: It was discussed when error.code didn't exist at the time (that is what I understood from this discussion: https://github.com/nodejs/node/pull/4664/files#diff-bbed55d3167db7c16baf3ee2c51acb1b09f7740ab1399ab015226c3df488753aR46). So it's not a fair discussion.

[joyeecheung]

The reason why we have this entire error.code system (and a former strategic initiative involving years of work assigning these codes) was to prevent having to treat error.message changes as server major. Only errors that are known to have their messages captured in the user land (mostly system errors) can still get the semver major treatment but otherwise error message changes are considered semver patch.

[marco-ippolito]

I believe @mcollina mentioned output changes in console were semver major.
I'm not sure the comparison it's fair between error.message and console.table,
console.table is widely used for benchmarks and snapshots, in documentation etc..., the output actually matters.
but I'm not gonna insist if it's something documented

[mcollina]

output changes can breaking. Maybe let's await backporting to LTS for a bit?

[mcollina]

This should likely not have been backported to v20, it broke our tests.
It also broke Undici tests.

[marco-ippolito]

This should likely not have been backported to v20, it broke our tests.
It also broke Undici tests.

Can we clearly add to the documentation that console output changes should be semver major?