string_decoder: refactor to use private properties

[anonrig]

Mainly the changes contain converting decoder into a class and use private properties for brand validation. It comes with a caveat that I want to ask the community feedback and help.

With the following changes, the following test does not work, and is removed. Therefore, it makes this pull request a breaking change? Can we avoid that? I'm adding needs-cigtm just in case...

// Should work without 'new' keyword
const decoder2 = {};
StringDecoder.call(decoder2);
assert.strictEqual(decoder2.encoding, 'utf8');

[cjihrig]

Therefore, it makes this pull request a breaking change? Can we avoid that?

I don't think so. That has previously stopped other things in core from being converted to a class.

[anonrig]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1326/

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3153/

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[targos]

Why did you abort the benchmark?

[anonrig]

Why did you abort the benchmark?

@targos There was a more urgent benchmark at that time. I'll re-run it after I fix the errors in this current pull request, and CIGTM was aborted due to Node 20.

[nodejs-github-bot]

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

[jasnell]

Unfortunately I think we need to take this through a proper runtime deprecation cycle before we can make this change. There are older versions of iconv-lite, and I'm sure other modules that this will break.