Support running karma-esm from a subdir

[nicojs]

Hi again 👋

I've dived into the karma-esm world and found the actual issue why Stryker isn't working with the this plugin.

It seems that it isn't possible to run karma-esm from within in a sub dir. I've created an example repo here.

Repro

git clone git@github.com:nicojs/karma-esm-from-a-subdir.git
cd karma-esm-from-a-subdir
npm i
cd foo/bar
npx karma start

This error is logged:

Uncaught TypeError: Failed to resolve module specifier "@open-wc/testing-helpers/index.js". Relative references must start with either "/", "./", or "../".
  at http://localhost:9876/context.html:0:0

Explanation

My dir structure is:

root
├── foo
│   └── bar
│       ├── node_modules (symlink -> ../../node_modules)
│       ├── karma.conf.js
│       ├── src
│       │   └── wc-test.js
│       ├── test
│       │   └── wc-test.test.js
│       └── karma.conf.js
└── node_modules

I figured out that a babel plugin is used to transform import statements to their true server path here:

open-wc/packages/karma-esm/src/forked-babel-plugin-bare-import-rewrite.js

Lines 92 to 119 in 01445cf

	const importPathAbs = absResolve(importPath, sourceFileName, pluginOptions);
	const nodeModules = path.resolve(pluginOptions.rootBaseDir || process.cwd(), 'node_modules');
	const isNodeModule = pathIsInside(importPathAbs, nodeModules);
	const fromNodeModule = pathIsInside(path.resolve(sourceFileName), nodeModules);
	let importPathRel = path.relative(path.dirname(sourceFileName), importPathAbs);
	const sep = pluginOptions.fsPath === true ? path.sep : path.posix.sep;
	// Disabled in fork
	// if (isNodeModule && !fromNodeModule) {
	// const modulesDir = pluginOptions.modulesDir || '/node_modules';
	// if (modulesDir.includes('://')) {
	// return modulesDir + (modulesDir.endsWith('/') ? '' : '/') + pathToURL(path.relative(nodeModules, importPathAbs));
	// }
	// importPathRel = path.join(
	// pluginOptions.modulesDir || '/node_modules',
	// path.relative(nodeModules, importPathAbs));
	// }
	if (pluginOptions.fsPath !== true) {
	importPathRel = pathToURL(importPathRel);
	}
	if (!isPathSpecifier(importPathRel) && !path.isAbsolute(importPathRel)) {
	importPathRel = '.' + sep + importPathRel;
	}
	return importPathRel;

This piece of code transforms import { html, fixture, expect } from '@open-wc/testing' to import { html, fixture, expect } from '../../../node_modules/@open-wc/testing'. This path is correct relative to the test file on the server. The problem is that the browser cannot make sense of it. The file that loads it has this URL: base/test/wc-test.test.js, so the url would be base/test/../../../node_modules/@open-wc/testing which is not supported.

Proposed solution

I've been able to fix this issue by allowing for symlinked node modules. I did this by changing two lines of code:

open-wc/packages/karma-esm/src/forked-babel-plugin-bare-import-rewrite.js

Line 79 in 01445cf

return require.resolve(result);

Change to result (remove require.resolve)

open-wc/packages/karma-esm/src/forked-babel-plugin-bare-import-rewrite.js

Line 71 in 01445cf

preserveSymlinks: false,

Change to preserveSymlinks: true

Having those 2 changes in place, '@open-wc/testing' is now transformed to '../node_modules/@open-wc/testing', which works (because the node_modules are symlinked there). This is because of change 1. Subsequent imports from inside the symlinked node_modules also work because of the preserveSymlinks flag.

The question is, would it be OK to change these 2 things? I've seen in the history there is some back-and-forth in this file and the karma-esm package doens't seem to have any tests associated with it.

[LarsDenBakker]

We explicitly added this behavior in this commit

The reason is that if we don't resolve symlinks literally there could be multiple files importing the same module by different paths, causing the same module to be loaded twice. This a common use case for monorepositories: #459

At the least we could make the symlink behavior configurable, but I'm wondering if we chose the wrong default here. In general karma-esm should not behave any different than a regular web server would.

Import maps will give us the control we need for complex use cases, so I feel that we should revert to make your proposed solution the default and add a resolveSymlinks option. What do you think @daKmoR ?

[nicojs]

I feel that we should revert to make your proposed solution the default and add a resolveSymlinks option

I'm all for it. I would suggest also adding an integration test with the setup we want to support, so this won't break in the future. I could look into creating the PR if you want, but it might take some time...

I didn't know about import maps yet. It would be great if those would land in browsers. Just for clarification, that would reduce the need for runtime parsing + changing bare import statements using babel, instead just telling the browser where he can find the bare imports?

[LarsDenBakker]

Yeah so we will be able to run code without any transformation.

I would like to have integration tests for this, but its quite a complex setup with stuff that runs in node and in the browser. Karma is quite a beast too.

Basically the open wc repo is the integration test right now.

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[LarsDenBakker]

This is still something we want to support

[ciskavs]

Hi,

I am trying to use Stryker for mutation testing our webcomponents, but run into the same issue: it fails on a dynamic import with the error "TypeError: Failed to resolve module specifier 'my-component/elements/my-code.js'". From this issue it is not clear to me what the solution was. Do you have any suggestions on how to fix it?