monounity / karma-typescript

Simplifying running unit tests with coverage for Typescript projects.
314 stars 109 forks source link

Dependency with `type: module` loads module as unevaluated text contents of `main` field #491

Open thw0rted opened 3 years ago

thw0rted commented 3 years ago

Repro here

I use the cesium library. It's written in vanilla ES6 JS, and they build a CJS version (two, actually, minified and unminified) to include in the NPM package as well. Their package.json file includes

  type: "module",
  main: "index.cjs",
  module: "Source/Cesium.js" // the ES6 module entry point

When I import { JulianDate } from "cesium", the imported object is undefined. Stepping through the debug test runner, I see that the transpiled require("cesium") call returns the literal text contents of index.cjs as a string, so of course the JulianDate property of that "module" is undefined.

In the short term I'm able to work around this with

        resolve: {
            alias: {
                "cesium": "node_modules/cesium/Build/Cesium/Cesium.js"
            }
        },

In the long run, I think you're going to see more packages shipping both ESM and CJS distros, and using package.json fields to provide the appropriate module type, so I figured I should report it.

rinick commented 2 years ago

I have the same issue when importing idb

https://github.com/jakearchibald/idb/blob/main/package.json

and in my case I'm not able to fix it with resolve:{...}

@thw0rted did you add that in package.json?

thw0rted commented 2 years ago

Sorry @rinick , I actually wound up being unable to use this package and went back to karma-webpack. It's a bummer because it means every change I make results in a webpack rebuild, so the test/edit/test cycle takes a pretty long time to run.

It's been a few months since I looked at this, but IIRC the issue that killed me was Angular support. It might have been that I couldn't get karma-typescript to work with @ngneat/spectator. I think I filed issues with both projects asking if they could help me get them working together, and I couldn't get traction on either side? I've since stopped using Spectator altogether so many when I have some spare cycles I'll try to get karma-typescript working again.

Anyway, your issue is slightly different from mine. cesium ships with ES6 (vanilla JS) sources and also a CJS bundle -- the resolve.alias entry (under Karma's bundlerOptions I think?) just forces it to use the CJS bundle instead of the ES6 source. I haven't checked the built package for idb but I don't see any builds that create ESM output in the rollup config file there.

It might help illustrate your problem if you can put together a minimal reproduction? (Also might help track down how/why this is happening in the first place...)

rinick commented 2 years ago

@thw0rted thanks for the reply. for now I just make a hard copy of all the .ts file I need from idb package as a quick work around. I might also move to karma-webpack when I have time.

thw0rted commented 2 years ago

You can certainly consider it, but if karma-ts works for you other than this one little issue, it's probably worth trying to stick with it. I believe with karma-webpack, you basically have webpack watching your files to make new builds, then Karma watching the webpack build to run new tests, while with karma-typescript you have karma watching the code more or less directly (with a transpiler in the middle). I don't have a big non-Angular project set up to do a side by side test, but I was trying to migrate from karma-webpack to karma-typescript because the former seems especially slow.

MarkHerhold commented 2 years ago

Hi there, I am also running into this issue but for a different dependency. What is the ideal fix to solve this issue - what specifically would need implemented in karma-typescript?

Finesse commented 2 years ago

@thw0rted I've faced this problem too with the fflate package. Here is a small reproducible demo: karma-typescript-491.zip (unpack the directory and run npm install && npm test inside).

The problem can be fixed by changing this karma-typescript line:

public isScript(): boolean {
-   return (this.filename && !this.isTypingsFile() && /\.(js|jsx|mjs|ts|tsx)$/.test(this.filename))
+   return (this.filename && !this.isTypingsFile() && /\.(js|jsx|mjs|cjs|ts|tsx)$/.test(this.filename))
        || this.transformedScript;
}

I don't make a PR because I'm not sure that this is a proper solution.


My current workaround to make fflate work is a special bundlerOptions value:

{
  // ...
  karmaTypescriptConfig: {
    // ...
    bundlerOptions: {
      exclude: ['worker_threads'],
      resolve: {
        alias: {
          fflate: 'node_modules/fflate/umd/index.js',
        },
      },
    },
  },
}