src,doc: refactor to replace typedefs with usings

[RaisinTen]

No description provided.

[mhdawson]

What level of compiler support does this syntax require?

[RaisinTen]

@mhdawson all compilers support this except except Digital Mars C++.
See row 11 (Template aliases) of: https://en.cppreference.com/w/cpp/compiler_support#cpp11

How do I fix the lint error? Running this doesn't change anything for me locally:

❯ npm run lint:fix

> [email protected] lint:fix
> git-clang-format '*.h', '*.cc'

no modified files to format

[mhdawson]

One more question, is there anything that you can do with a typedef definition that you can't do with the using def ?

[RaisinTen]

@mhdawson no there isn't.
I preferred using using here because it can do whatever typedef does + it also allows templates.

[legendecas]

How do I fix the lint error? Running this doesn't change anything for me locally:

The lint is comparing the local changes with the main branch. Please have a check on your local main branch and dev branch are up to date with the GitHub one.

[RaisinTen]

The lint is comparing the local changes with the main branch. Please have a check on your local main branch and dev branch are up to date with the GitHub one.

@legendecas Yes, they are up to date. Turns out, getNativeBinary() of node_modules/clang-format/index.js can't find the clang-format binary on my machine because it's an unsupported architecture I guess (ia32). tools/clang-format.js ends up swallowing the error because it checks for error instead of stderr of the spawned ChildProcess.

I tried to copy and paste the linter error log into a file and git apply it but unfortunately it prints:

error: corrupt patch at line 48

:/

[RaisinTen]

PR to fix the error detection: #914

[RaisinTen]

I could apply the patch from the logs of the failed style check run and now the style check passes too.

However, it seems to have added some more changes than what I intended (like indenting some other parts of the code).
Would it make sense to run clang-format on the full codebase and make a PR to fix the style so that other PRs don't have to make unrelated style changes?

PTAL.

[mhdawson]

LGTM

[mhdawson]

@nodejs/node-api let me know if you have any concerns about this one by Friday (or approve), otherwise I'll go ahead and land it.

[NickNaso]

Some notes about the reasons that pushed me to approve this PR:

It's possible to define a new name for an existing type with both:

• typedef (We call this declaration typedef and the resulting name typedef-name)
• using (We can call this decalration alias desclaration)

The using alternative is more readable because you always have the type name on the left side of the =.

Unlike a typedef an alias declaration can be templated to provide a conveninet name for a family of types. This is available since C++11 and is called alias template.

[mhdawson]

Need to run on our CI to make sure all the older compilers support using before landing.

[NickNaso]

CI:

Version	Job	Status
v16.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3744/	✓
v15.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3743/	✓
v14.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3742/	✓
v13.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3741/	✓
v12.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3740/	✓
v11.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3739/	X
v10.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/3738/	✓

The test fails also with the main branch for Node.js v11.x
@mhdawson I tried to run the CI and reported the results here. If you have time please take a look.

[mhdawson]

@NickNaso thanks for running the CIs

The failures on 11 seem unrelated to this PR as you mention, possibly because of something that would have been backported to 10.x but not 11.

Checking the table in https://en.cppreference.com/w/cpp/compiler_support#cpp11 it also seems to confirm that all of the minimum levels for Node.js 10.x support using so I think this is good to land.

[mhdawson]

Landed as 71494a4