BREAKING: Map-like interface for Deno.env (#4942)

9 files changed, 53 insertions, 58 deletions

diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts
index edc9668e7..e58ec0b55 100644
--- a/cli/js/lib.deno.ns.d.ts
+++ b/cli/js/lib.deno.ns.d.ts
@@ -116,29 +116,35 @@ declare namespace Deno {
    */
   export function exit(code?: number): never;
 
-  /** Returns a snapshot of the environment variables at invocation. Changing a
-   * property in the object will set that variable in the environment for the
-   * process. The environment object will only accept `string`s as values.
-   *
-   *       const myEnv = Deno.env();
-   *       console.log(myEnv.SHELL);
-   *       myEnv.TEST_VAR = "HELLO";
-   *       const newEnv = Deno.env();
-   *       console.log(myEnv.TEST_VAR === newEnv.TEST_VAR);  // outputs "true"
-   *
-   * Requires `allow-env` permission. */
-  export function env(): {
-    [index: string]: string;
-  };
+  export const env: {
+    /** Retrieve the value of an environment variable. Returns undefined if that
+     * key doesn't exist.
+     *
+     *       console.log(Deno.env.get("HOME"));  // e.g. outputs "/home/alice"
+     *       console.log(Deno.env.get("MADE_UP_VAR"));  // outputs "Undefined"
+     *
+     * Requires `allow-env` permission. */
+    get(key: string): string | undefined;
 
-  /** Retrieve the value of an environment variable. Returns undefined if that
-   * key doesn't exist.
-   *
-   *       console.log(Deno.env("HOME"));  // e.g. outputs "/home/alice"
-   *       console.log(Deno.env("MADE_UP_VAR"));  // outputs "Undefined"
-   *
-   * Requires `allow-env` permission. */
-  export function env(key: string): string | undefined;
+    /** Set the value of an environment variable.
+     *
+     *       Deno.env.set("SOME_VAR", "Value"));
+     *       Deno.env.get("SOME_VAR");  // outputs "Value"
+     *
+     * Requires `allow-env` permission. */
+    set(key: string, value: string): void;
+
+    /** Returns a snapshot of the environment variables at invocation.
+     *
+     *       Deno.env.set("TEST_VAR", "A");
+     *       const myEnv = Deno.env.toObject();
+     *       console.log(myEnv.SHELL);
+     *       Deno.env.set("TEST_VAR", "B");
+     *       console.log(myEnv.TEST_VAR);  // outputs "A"
+     *
+     * Requires `allow-env` permission. */
+    toObject(): { [index: string]: string };
+  };
 
   /** **UNSTABLE** */
   export type DirKind =
diff --git a/cli/js/ops/os.ts b/cli/js/ops/os.ts
index e01718c8c..30aa6d0d4 100644
--- a/cli/js/ops/os.ts
+++ b/cli/js/ops/os.ts
@@ -27,22 +27,13 @@ function getEnv(key: string): string | undefined {
   return sendSync("op_get_env", { key })[0];
 }
 
-export function env(): { [index: string]: string };
-export function env(key: string): string | undefined;
-export function env(
-  key?: string
-): { [index: string]: string } | string | undefined {
-  if (key) {
-    return getEnv(key);
-  }
-  const env = sendSync("op_env");
-  return new Proxy(env, {
-    set(obj, prop: string, value: string): boolean {
-      setEnv(prop, value);
-      return Reflect.set(obj, prop, value);
-    },
-  });
-}
+export const env = {
+  get: getEnv,
+  toObject(): { [key: string]: string } {
+    return sendSync("op_env");
+  },
+  set: setEnv,
+};
 
 type DirKind =
   | "home"
diff --git a/cli/js/tests/os_test.ts b/cli/js/tests/os_test.ts
index 58dcd1bc5..a44b69e7d 100644
--- a/cli/js/tests/os_test.ts
+++ b/cli/js/tests/os_test.ts
@@ -8,24 +8,22 @@ import {
 } from "./test_util.ts";
 
 unitTest({ perms: { env: true } }, function envSuccess(): void {
-  const env = Deno.env();
-  assert(env !== null);
-  // eslint-disable-next-line @typescript-eslint/camelcase
-  env.test_var = "Hello World";
-  const newEnv = Deno.env();
-  assertEquals(env.test_var, newEnv.test_var);
-  assertEquals(Deno.env("test_var"), env.test_var);
+  Deno.env.set("TEST_VAR", "A");
+  const env = Deno.env.toObject();
+  Deno.env.set("TEST_VAR", "B");
+  assertEquals(env["TEST_VAR"], "A");
+  assertNotEquals(Deno.env.get("TEST_VAR"), env["TEST_VAR"]);
 });
 
 unitTest({ perms: { env: true } }, function envNotFound(): void {
-  const r = Deno.env("env_var_does_not_exist!");
+  const r = Deno.env.get("env_var_does_not_exist!");
   assertEquals(r, undefined);
 });
 
 unitTest(function envPermissionDenied1(): void {
   let err;
   try {
-    Deno.env();
+    Deno.env.toObject();
   } catch (e) {
     err = e;
   }
@@ -37,7 +35,7 @@ unitTest(function envPermissionDenied1(): void {
 unitTest(function envPermissionDenied2(): void {
   let err;
   try {
-    Deno.env("PATH");
+    Deno.env.get("PATH");
   } catch (e) {
     err = e;
   }
@@ -62,7 +60,7 @@ unitTest(
     ): Promise<void> => {
       const src = `
       console.log(
-        ${JSON.stringify(Object.keys(expectedEnv))}.map(k => Deno.env(k))
+        ${JSON.stringify(Object.keys(expectedEnv))}.map(k => Deno.env.get(k))
       )`;
       const proc = Deno.run({
         cmd: [Deno.execPath(), "eval", src],
@@ -79,8 +77,8 @@ unitTest(
       proc.close();
     };
 
-    assertEquals(Deno.env("path"), Deno.env("PATH"));
-    assertEquals(Deno.env("Path"), Deno.env("PATH"));
+    assertEquals(Deno.env.get("path"), Deno.env.get("PATH"));
+    assertEquals(Deno.env.get("Path"), Deno.env.get("PATH"));
 
     // Check 'foo', 'Foo' and 'Foo' are case folded.
     await checkChildEnv({ foo: "X" }, { foo: "X", Foo: "X", FOO: "X" });
diff --git a/cli/tests/permission_test.ts b/cli/tests/permission_test.ts
index 1e6abae21..bcfb840bf 100644
--- a/cli/tests/permission_test.ts
+++ b/cli/tests/permission_test.ts
@@ -11,7 +11,7 @@ const test: { [key: string]: Function } = {
     makeTempDirSync();
   },
   envRequired(): void {
-    env().home;
+    env.get("home");
   },
   netRequired(): void {
     listen({ transport: "tcp", port: 4541 });
diff --git a/std/examples/gist.ts b/std/examples/gist.ts
index c41b9b98e..fbe4384e1 100755
--- a/std/examples/gist.ts
+++ b/std/examples/gist.ts
@@ -7,7 +7,7 @@ function pathBase(p: string): string {
   return parts[parts.length - 1];
 }
 
-const token = Deno.env()["GIST_TOKEN"];
+const token = Deno.env.get("GIST_TOKEN");
 if (!token) {
   console.error("GIST_TOKEN environmental variable not set.");
   console.error("Get a token here: https://github.com/settings/tokens");
diff --git a/std/node/module.ts b/std/node/module.ts
index b7a0107d0..9203764f0 100644
--- a/std/node/module.ts
+++ b/std/node/module.ts
@@ -534,8 +534,8 @@ class Module {
   }
 
   static _initPaths(): void {
-    const homeDir = Deno.env("HOME");
-    const nodePath = Deno.env("NODE_PATH");
+    const homeDir = Deno.env.get("HOME");
+    const nodePath = Deno.env.get("NODE_PATH");
 
     // Removed $PREFIX/bin/node case
 
diff --git a/std/node/process.ts b/std/node/process.ts
index 310a7e814..f90e1eada 100644
--- a/std/node/process.ts
+++ b/std/node/process.ts
@@ -30,7 +30,7 @@ export const process = {
   on,
   get env(): { [index: string]: string } {
     // using getter to avoid --allow-env unless it's used
-    return Deno.env();
+    return Deno.env.toObject();
   },
   get argv(): string[] {
     // Deno.execPath() also requires --allow-env
diff --git a/std/node/process_test.ts b/std/node/process_test.ts
index b9d5388ea..3afaa4cdf 100644
--- a/std/node/process_test.ts
+++ b/std/node/process_test.ts
@@ -3,7 +3,7 @@ import { assert, assertThrows, assertEquals } from "../testing/asserts.ts";
 import { process } from "./process.ts";
 
 // NOTE: Deno.execPath() (and thus process.argv) currently requires --allow-env
-// (Also Deno.env() (and process.env) requires --allow-env but it's more obvious)
+// (Also Deno.env.toObject() (and process.env) requires --allow-env but it's more obvious)
 
 test({
   name: "process.cwd and process.chdir success",
diff --git a/std/path/win32.ts b/std/path/win32.ts
index d4febf706..9bba66e2b 100644
--- a/std/path/win32.ts
+++ b/std/path/win32.ts
@@ -39,7 +39,7 @@ export function resolve(...pathSegments: string[]): string {
       // absolute path, get cwd for that drive, or the process cwd if
       // the drive cwd is not available. We're sure the device is not
       // a UNC path at this points, because UNC paths are always absolute.
-      path = env()[`=${resolvedDevice}`] || cwd();
+      path = env.get(`=${resolvedDevice}`) || cwd();
 
       // Verify that a cwd was found and that it actually points
       // to our drive. If not, default to the drive's root.