cspotcode / tsimportlib

MIT License
11 stars 4 forks source link

Can't import node-fetch in vitest #8

Open Maxim-Mazurok opened 1 year ago

Maxim-Mazurok commented 1 year ago
    ×  nx run doppler:test
        RUN  v0.31.0 C:/Users/maxim.mazurok/transactions/packages/doppler

 ❯ src/lib/doppler.integration.spec.ts  (1 test | 1 failed) 17ms
   ❯ src/lib/doppler.integration.spec.ts > should work
     → Unable to locate module "node-fetch" relative to "undefined" using the CommonJS resolver.  Consider passing an absolute path to the target module.

 Test Files  1 failed (1)
      Tests  1 failed (1)
   Start at  08:06:02
   Duration  2.81s (transform 741ms, setup 404ms, collect 105ms, tests 17ms, environment 0ms, prepare 801ms)

⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/lib/doppler.integration.spec.ts > should work
Error: Unable to locate module "node-fetch" relative to "undefined" using the CommonJS resolver.  Consider passing an absolute path to the target module.
 ❯ Proxy.importEsm ../../node_modules/tsimportlib/index.js:22:15
 ❯ Module.setEnvironment src/lib/doppler.ts:8:46
      6| export const setEnvironment = async () => {
      7|   // eslint-disable-next-line @typescript-eslint/consistent-type-asser…
      8|   const { default: fetch, Headers } = (await dynamicImport(
       |                                              ^
      9|     "node-fetch",
     10|     module,
 ❯ src/lib/doppler.integration.spec.ts:8:9

My code:

import { execSync } from "node:child_process";
import { dynamicImport } from "tsimportlib";
import { z } from "zod";

// inspired by https://docs.doppler.com/docs/sdk-javascript#async
export const setEnvironment = async () => {
  // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
  const { default: fetch, Headers } = (await dynamicImport(
    "node-fetch",
    module,
  )) as typeof import("node-fetch");

  // see https://docs.doppler.com/docs/cli#fetch-cli-token-from-your-local-environment
  const DOPPLER_TOKEN = execSync("doppler configure get token --plain")
    .toString()
    .trim();

  const headers = new Headers({
    Authorization: `Bearer ${DOPPLER_TOKEN}`,
  });

  const response = await (
    await fetch(
      "https://api.doppler.com/v3/configs/config/secrets/download?format=json",
      { headers },
    )
  ).json();

  const environmentVariables = z.record(z.string()).parse(response);

  for (const [key, value] of Object.entries(environmentVariables)) {
    process.env[key] = value;
  }
};

My test:

import { setEnvironment } from "./doppler";

it("should work", async () => {
  // Arrange
  const oldEnvironmentKeys = Object.keys(process.env);

  // Act
  await setEnvironment();

  // Assert
  const newEnvironmentKeys = Object.keys(process.env);
  const newKeys = newEnvironmentKeys.filter(
    (x) => !oldEnvironmentKeys.includes(x),
  );
  expect(oldEnvironmentKeys).not.toEqual(newEnvironmentKeys);
  expect(newKeys.sort()).toStrictEqual(
    [
      "BLA",
    ].sort(),
  );
});
Maxim-Mazurok commented 1 year ago

The following patch works for me:

Looks like we don't need any logic at all, just import works fine.
Also, with original logic vitest in nx fails to import `node-fetch`.
See https://github.com/cspotcode/tsimportlib/issues/8

diff --git a/node_modules/tsimportlib/index.d.ts b/node_modules/tsimportlib/index.d.ts
index 369a3a6..dbf0199 100644
--- a/node_modules/tsimportlib/index.d.ts
+++ b/node_modules/tsimportlib/index.d.ts
@@ -1,18 +1,12 @@
-export interface MinimalNodeModule {
-    filename: string;
-}
-
 /**
  * Asynchronously import a native ESM module from within a `.ts` file compiled to CommonJS.
  *
  * Usage:
  *
  * ```
- * await dynamicImport(module, 'native-esm-module');
+ * await dynamicImport('native-esm-module');
  * ```
- *
- * @param module node `module` object for the importing file
  */
-export function dynamicImport(importSpecifier: string, module: MinimalNodeModule): Promise<any>;
+export function dynamicImport(importSpecifier: string): Promise<any>;

 export {dynamicImport as importEsm};
diff --git a/node_modules/tsimportlib/index.js b/node_modules/tsimportlib/index.js
index e5096d6..47bbb5d 100644
--- a/node_modules/tsimportlib/index.js
+++ b/node_modules/tsimportlib/index.js
@@ -1,25 +1,6 @@
-const Module = require('module');
-const {isAbsolute} = require('path');
-const {pathToFileURL} = require('url');
-
 exports.dynamicImport = importEsm;
 exports.importEsm = importEsm;

-async function importEsm(specifier, module) {
-    if(isAbsolute(specifier)) {
-        return import(pathToFileURL(specifier).href);
-    }
-    let resolvedPath;
-    try {
-        const req = Module.createRequire(module.filename);
-        try {
-            resolvedPath = req.resolve(Path.posix.join(specifier, 'package.json'));
-        } catch {
-            resolvedPath = req.resolve(specifier);
-        }
-        resolvedPath = pathToFileURL(resolvedPath).href;
-    } catch {
-        throw new Error(`Unable to locate module "${specifier}" relative to "${module?.filename}" using the CommonJS resolver.  Consider passing an absolute path to the target module.`);
-    }
-    return import(resolvedPath);
+async function importEsm(specifier) {
+    return import(specifier);
 }
SnowMarble commented 1 year ago

If the code runs fine without being called from the test code, I think you should check whether your test environment supports ES modules.

And the problem in your patch is that it can't import es module by relative path. Given a relative path, it will look for files at its location, so that's why we need resolvedPath.

Please check your test environment :)

Maxim-Mazurok commented 1 year ago

I think the code was working fine outside of the testing environment, yes.

I'm using the latest vitest - 0.31.4, which is supposed to support ESM out of the box.

All of my files/packages are CJS, so I guess I have no need for relative paths.

I guess my main goal here was to share what works for me, so hopefully it will help someone. I don't really need this to be addressed as the patch seems to work and I'm pretty happy with it. So feel free to close as not planned if you'd like, cheers!