GeezerVison

Some decisions probably need to be made re: functions and objects. Neither is technically a primitive.

@apapirovski Agreed. But most of the codes using function and object instead of uppercase initial. I'm also confused with the strange convention.

Defensively marking as semver-major... these type of changes should only need to be semver-major while we are still in the process of converting the errors over to the new mechanism.

Not sure how I feel about the need for checkExpectedTypeInitial — feels like reviewing PRs and modifying all existing usage would take care of its purpose.

👍

Defensively marking as semver-major... these type of changes should only need to be semver-major while we are still in the process of converting the errors over to the new mechanism.

@jasnell A lot of changes here only affect the error message. If this PR can split those changes touching error codes/adding additional throws out then IMO we could land the message-only changes as semver-patch.

Some decisions probably need to be made re: functions and objects. Neither is technically a primitive.

@apapirovski I'd say...let's go with util.isPrimitive.

> util.isPrimitive(() => {})
false
> util.isPrimitive({})
false

@joyeecheung yeah, I agree — more about whether we capitalize or not. I feel like Function looks a bit weird but it's not a primitive so...

Can you split the message-only changes into separate commits? (I guess we need to split the PR as well) That way they don't have to be semver-major.

@joyeecheung Sorry I don't understand fully. Do you mean that split every file change (about error message) into separate commits? It will be a lot of commits I guess.

Personally I prefer not to check the capitalizations of types in runtime, we can probably just make another eslint rule for it.

Agreed. I will remove it.

@starkwang Sorry for not being clear enough. Some of the changes here do not add additional throw or change the error code (i.e. do not change the first argument like "ERR_*", basically they just change the capitalization, so only the errror messages are changed), those can be split into one commit, and do not have to be sermver-major(that is, can go into 8.x). Those that do change the code or add additional throws can be split into another commit and will have to wait until 9.x

@joyeecheung I've just reduced this PR into message-only changes, which unifies the initials.
I will create another PR to fix the inappropriate error codes (they were in this PR before).

LGTM once this changes object -> Object and function -> Function, because we use the capitalized names in the documentation as well, IMO we should be more consistent on this.

Also I think I have incorrectly estimated the timeline in #16401 (comment) , from a quick glance I think none of the errors changed here can be backported in v8.x becuase they have not been migrated there in April anyway. The changes in this PR should be safe to land in v9.x and the other semver-major changes would probably need to wait for v10.x because we are very close to cut v9.x now.

Pushed commit to address comment.

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

Personally, I usually use lowercase for things which typeof returns a correct value for, e.g. number, function and object, but I guess consistency with the documentation makes sense.

@jasnell @joyeecheung @tniessen Is there anyone can review or land this PR?

This is a message-only change on the migrated errors now so we don't need TSC approvals...going to land this tomorrow if no one objects. cc @nodejs/collaborators

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

for me, the types with actual casing that a compiler can successfully parse is one thing, and the types that a consumer can easily relate to is another thing, and it makes sense to abstract the strict syntax in human readable messages. However, consistency draws the bottom line.

Should this be backported to v6.x-staging or v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

As this PR touches more than 50 files it may have been nice to get a backport lined up when this landed

@MylesBorins This PR cannot land in v6.x or v8.x because it changes errors that have not been migrated in either branches, see #16401 (comment) . This should be possible to be backported to v9.x though, if it actually needs a backport.

@MylesBorins Ah nevermind, I see that it's already released on v9.2.0