feature request: option to disable TEXT_ENCODING env check

[andykais]

Hi, I am using this library in deno, and I noticed part of the encode/decode flow checks for the TEXT_ENCODING environment variable. It seems like this is a useful env var for testing, but I do not need it in production. In deno, I have to allow access to this var explicitly, and will have to tell users of my library to do the same, which feels confusing if this is in fact a testing var. I was wondering if it would be possible to send along a flag like

import * as msgpack from 'npm:@msgpack/msgpack'

msgpack.decode(buffer, { env_check: false })
// or
msgpack.decode(buffer, { text_encoding: false })

for reference, this is the warning deno will share when I do not allow access to env with msgpack.

./test/functional.test.ts (uncaught error)
error: PermissionDenied: Requires env access to "TEXT_ENCODING", run again with the --allow-env flag
    return Deno.env.get(name);
                    ^
    at Object.getEnv [as get] (deno:runtime/js/30_os.js:86:16)
    at denoEnvGet (https://deno.land/[email protected]/node/_process/process.ts:38:21)
    at Object.get (https://deno.land/[email protected]/node/_process/process.ts:59:24)
    at Object.<anonymous> (file:///home/andrew/.cache/deno/npm/registry.npmjs.org/@msgpack/msgpack/2.8.0/dist/utils/utf8.js:7:177)
    at Object.<anonymous> (file:///home/andrew/.cache/deno/npm/registry.npmjs.org/@msgpack/msgpack/2.8.0/dist/utils/utf8.js:168:4)

None of this is really a blocker for using this library, I just believe it could make usage a bit more friendly for the deno audience :)

[gfx]

Thank you for your report. That's a good point. I would rather like to surround the env access with try { ... } to ignore permission errors in Deno (or any other JavaScript runtimes). Does it work for you?

[andykais]

hmm that will still prompt the user for access to the var (unless they specifically pass --no-prompt to deno, which will just error out). What is the purpose of the env var?

[gfx]

Ah, that's a good point. So I'm willing to implement the feature to omit env accesses.

FYI the env accesses are required to test the cases where TextEncoder is not implemented (https://caniuse.com/textencoder it's only IE!). Because I'm dropping IE support, removing the branch where TextEncoder is not implemented might be good enough.

[andykais]

👍 well that would be good for me, I trust you will do whats best for your library! You could always drill down an option bag rather than use env vars like I had suggested in the initial message if you dont want to remove it entirely (I feel like nodejs still doesnt have a TextEncoder class?)