feat: 🎸 handle environment variable access

[dancrumb]

I did a little refactoring as well as the changes needed for supporting environment variables.

I'm happy to rollback the reactors if you don't like them; just let me know!

✅ Closes: #103

[garronej]

Hello @dancrumb,

Firstly, I want to express my appreciation for the high standard of this Pull Request. It is clearly evident that a great deal of meticulous work and thought went into it. Your decision to refactor certain parts of the codebase has also been recognized and is greatly appreciated; this effort will undeniably enhance our project's overall readability.

However, I wanted to discuss a particular change you've made. I noticed that the compilation target has been altered in your PR. Could you please help me understand your rationale behind this decision? My concern is that this modification might potentially render Denoify incompatible with older versions of Node.js. We'd certainly want to avoid this, as it may impact users who are yet to upgrade to newer Node.js versions.

Thank you in advance for addressing this concern. I'm eager to further understand your thought process.

[dancrumb]

Hi @garronej ,

I changed the target so that I could use String.replaceAll.

My rationale was the assumption that anyone using denoify ought to be in a position whereby they can be using Node 15+

I would agree that those folks might not want their own code built to this target, but it seemed reasonable to bump the denoify target since that's only ever run in a development environment.

On the other hand, I'd be happy to convert this to not use replaceAll. Since there is an alternative to this function, if you wish to keep the target unchanged, I'm happy to do that.

Let me know which part you'd prefer to take :)

[garronej]

Allright, you convinced me 👍🏻

[garronej]

Upon further analysis of the code, I was impressed by the simplicity and efficiency of replaceAll! I feel remiss for not having utilized it previously. My approach for accomplishing similar replacements in other projects seems rather more complicated by comparison.

I hope it's not too much to ask, but would you be open to revising your implementation so that the following unit test succeeds?

it("should accurately update dynamic environment variables", async () => {
    const sourceCode = `process.env["FOO"];process.env['FOO'];process.env["FO" + "O"];process.env[\`F${"O"}O\`]`;
    const expected = `Deno.env.get("FOO");Deno.env.get('FOO');Deno.env.get("FO" + "O");Deno.env.get(\`F${"O"}O\`)`;

    const { denoifySingleFile } = denoifySingleFileFactory({
        "denoifyImportExportStatement": () => {
            tsafeAssert(false);
        }
    });

    const modifiedSourceCode = await denoifySingleFile({
        sourceCode,
        "dirPath": ""
    });

    expect(modifiedSourceCode).toBe(expected);
});

I believe that it's fairly standard practice to read environment variables dynamically.

[dancrumb]

Agreed! I'll address that test case in my fix for #108