Update esm loader hooks API

[jonaskello]

This is an attempt to fix #1372.

Link to docs for the new hooks API:
https://github.com/nodejs/node/blob/master/doc/api/esm.md#loaders

EDIT from @cspotcode:

• If Feature: Expose esm hooks factory to public API #1439 is merged first, update createEsmHooks to expose the new hooks

[codecov]

Codecov Report

Merging #1457 (8b328d3) into main (4a0db31) will decrease coverage by 0.15%.
The diff coverage is 75.75%.

Impacted Files	Coverage Δ
dist-raw/node-esm-default-get-format.js	71.42% <71.42%> (ø)
src/esm.ts	84.21% <73.68%> (-3.93%)	⬇️
src/index.ts	77.83% <91.66%> (+0.27%)	⬆️
dist-raw/node-package-json-reader.js	88.88% <0.00%> (+5.55%)	⬆️

[cspotcode]

Thanks for the PR. I'm swamped with work, so I'll do my best to review in a timely manner, but it might drag a bit. However, anyone who wants to use this PR before it is merged and published can install directly from git. npm's documentation explains how to install dependencies directly from a git branch instead npmjs.com. Our CI also uploads a package tarball as an artifact, which anyone can download and install locally.

Re the warnings from node about deprecated hooks: I think we can detect the node version and conditionally export only the hooks expected by that version of node. We can export undefined for the others. We do version detection in a few places in the codebase; here is one:

ts-node/src/index.ts

Lines 43 to 62 in aaf6052

	/**
	* Does this version of node obey the package.json "type" field
	* and throw ERR_REQUIRE_ESM when attempting to require() an ESM modules.
	*/
	const engineSupportsPackageTypeField =
	parseInt(process.versions.node.split('.')[0], 10) >= 12;
	function versionGte(version: string, requirement: string) {
	const [major, minor, patch, extra] = version
	.split(/[\.-]/)
	.map((s) => parseInt(s, 10));
	const [reqMajor, reqMinor, reqPatch] = requirement
	.split('.')
	.map((s) => parseInt(s, 10));
	return (
	major > reqMajor ||
	(major === reqMajor &&
	(minor > reqMinor || (minor === reqMinor && patch >= reqPatch)))
	);
	}

[cspotcode]

Should we add node's nightly builds to our test matrix? We already test against nightly typescript.

[cspotcode]

Linking to actions/setup-node#80 re testing against node nightlies. If they don't have a good solution, we can roll our own I guess.

[cspotcode]

I've added node nightly to the test matrix which is causing some failures. It also looks like stack traces are not showing correct line numbers for the errors. I don't know if this is a ts-node bug, a node nightly bug, or if those stack traces have never been correct.

[jonaskello]

According to the docs it seems source return value for formats builtin and commonjs should be undefined. I added a check for this and I think it might fix the tests.

[jonaskello]

Well, it seemed to fix all the tests but one :-).

[cspotcode]

That's great; do you know what's up with the remaining failing test?

[cspotcode]

I'm pretty sure the remaining test failures can be fixed by updating esm/transpile-only.mjs to export the new hooks. With that, this is probably ready to merge.

[jonaskello]

Thanks, I'll try to get some time to look into the last test failure.

FYI, I'm also working towards a more complete solution for the resolve hook in a new package called ts-resolve. It is still in an early stage but it would of course be interesting to know if something like this package would be considered useful for ts-node?

[cspotcode]

The fix was a one-line change; tests are passing now. 63c0618

it would of course be interesting to know if something like this package would be considered useful for ts-node?

Possibly, yeah. We want to do both CJS and ESM resolution so that projects "just work," so if we support project references for ESM it should also work with CJS. And since we already resolve the user's configuration, we'd want it to accept a pre-parsed config or set of multiple project-referenced configs, share FS cache, stuff like that. We'll also need to be able to hook into format determination to override as dictated by https://typestrong.org/ts-node/docs/module-type-overrides

Glancing at the readme, looks like it ticks at least a few of those boxes.

[cspotcode]

Looks good to go. Follow-up PRs can fill in the appropriate version numbers once the new loader API lands in node.

[cspotcode]

Does esbuild's sync API still have that per-call process spawning overhead? That's why I implemented swc support for ts-node, but I want to add esbuild as well once that process-spawning limitation goes away.

[jonaskello]

The fix was a one-line change; tests are passing now.

Nice :-)

We want to do both CJS and ESM resolution...

The initial goal for ts-esm-resolve is for ESM resolution. Not sure how much work it would be to add CJS resolution. A lot of the logic is of course similar so it might be rather easy.

...accept a pre-parsed config or set of multiple project-referenced configs

Yes, currently it is accepting a path to a tsconfig file and resolving it to an internal Map of all referenced tsconfigs. Might be better to make that map a direct input param instead so pre-parsed config could be passed in.

...share FS cache

That should be possible since all FS calls are done through callback functions passed to the resolve function. I currently use this to mock the filesystem in the test-cases but it should also be possible to use it for cached FS calls.

[jonaskello]

...hook into format determination

AFAIK the resolve process does not need to know the format, that is only needed later when executing the module?

[cspotcode]

Oh yeah, you're right: format only needed for load not resolve.

Might be better to make that map a direct input param instead so pre-parsed config could be passed in.

We allow ts-node-specific overrides to the compilerOptions, so we'd need a way to do that. We already apply those overrides in-memory, so as long as we can relay that info to the target library, should work. Having said that, not sure what the use case would be for overriding "paths" for ts-node.

[jonaskello]

Does esbuild's sync API still have that per-call process spawning overhead? That's why I implemented swc support for ts-node, but I want to add esbuild as well once that process-spawning limitation goes away.

I haven't dug deep into the transpilers for load. I was investigating using snowpack or vitejs and they both use esbuild. However they do not resolve typescript files fully for project references. In the end I would like to have a full-stack typescript ESM setup with both node and snowpack/vitejs supporting full typescript resolving for references/workspaces. I've been experimenting with such a setup in this repo. Currently it uses the ts-node esm loader and snowpack.

[cspotcode]

Noting that I was concerned about some sourcemaps, but I can't remember why or if they're fixed or covered by tests.
#1457 (comment)

This is still an experimental API surface so I feel good merging now.