replace deps encoding with build in textdecoder

[Uzlopak]

We should discuss about this first. This replaces the jsencoding code with the built in textdecoder.

We have unfortunately no busboy benchmarks. Maybe we should add first some benchmarks before checking the validity of this PR.

I cache the instantiated TextDecoder in a Map because I expect that instantiating a TextDecoder is expensive and reusing it is cheap and it has a low memory footprint. Maybe my expectation is wrong?

Nice sideeffect, the test coverage increases massively. LOL

File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                                             
-------------------|---------|----------|---------|---------|---------------------------------------------------------------
All files          |   93.15 |    85.52 |   95.38 |   93.03 |                                                               

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[kibertoad]

I second the idea of having busboy benchmarks, we could do an informed decision here then.

[kibertoad]

no default? what will happen if we don't resolve anything there?

edit: right, we will use the passed one

[kibertoad]

do we need destEncoding outside of this block?

[Uzlopak]

no, we get from outside if it is 7bit or 8bit. But I think I made a mistake because this is actually the textEncoding and not the destEncoding

[kibertoad]

considering that all tests still passed, it's a very alarming sign. we do need to improve our code coverage for this part of functionality ahead of this change.

[Uzlopak]

I think it would be a huge task to do that, with little to no benefits. Or are there any nodejs unit tests for the builtin textdecoder, which we could reuse?

[kibertoad]

I am very scared of regressing edge cases here, considering both the size of original implementation and low code coverage of it. Wonder if we could transitively increase code coverage of encoder via writing additional tests for busboy itself first.

[Uzlopak]

I think that jsencoding dependency is actually an archaic TextDecoder Implementation.

But what i realized is, that if I output the error messages of the created try catch blocks, it actually throws errors in node 10 and 12 reporting that iso-8891-1, despite the unit test passes. Maybe because the files are utf-8 encoded?

    ✔ Multiple extended parameters (RFC 5987) with mixed charsets
RangeError [ERR_ENCODING_NOT_SUPPORTED]: The "iso-8859-1" encoding is not supported

So I think, the issue is, that the built in TextDecoder is depending on the built-in ICU.
https://nodejs.org/docs/latest-v14.x/api/util.html#util_encodings_supported_when_icu_is_disabled

So what we can do is, to try to get a built-in TextDecoder and if we throw we use the deps/encoding like before as fallback. Maybe if we split the encoders, and require them if necessary reduces the start up time of busboy. But only if it means, that we have some performance gains.

So we should postpone this PR till we have some busboy benchmarks.

[Uzlopak]

@kibertoad We could remove actually all encoders, as we dont need the encoders but only the decoders.

[Uzlopak]

I removed the encoders and extracted the big json array into encodings.json.

[Uzlopak]

I tested it with the benchmarks from #40 and it is surprisingly slower. So I would say, We should not use the built-in TextDecoder...