update node version (v18, v20, v22)

[doc-han]

Short Description

This PR ensures that Openfn/kit tooling works on several node versions. (v18, v20 & v22)

Fixes

#542
#617

Implementation Details

1. chore: move from ts-node to swc-loader (f857884)

Removes ts-node from the codebase and adds @swc-node/register as the new loader for running ava tests.

2. ci: update CI to run tests for v18, v20 & v22 (33f777d)

Updates the CI to run unit_tests and integration_tests under the node versions 18,20, 22.

3. fix: several tests & mocks (1222c9c)

This commit fixes several mocks that seem to function in v18 but not in other node versions. Some of them have to do with functionalities that don't exist across versions and so on.

1. Change implementation of mockSocket to use a class instead of a function.
2. Update the import of package.json from import with assert to use fs
3. Update wording of expected error cannot read properties of undefined to consider cannot destructure property
4. update test that waits for workerprocess to be killed to actually be killed. Because workerProcess.kill('SIGTERM') is async

4. fix: resolve mock-fs (fc2e5e1)

1. readFileSync with utf-8 was broken by an optimization in node. An update to [email protected] resolved that. (readFileSync with 'utf-8' option broken by node v20.5.0 tschaub/mock-fs#377)
2. dynamic import from a mocked filesystem stopped working with mock-fs. No good issue around that. Using the real filesystem resolved that. (Can't require files with mocked filesystem tschaub/mock-fs#130)

5. test: collection tests & flaky test (4cfd339)

1. A test issue introduced by tests accompanying collections.
2. skipping the test "allow a job to complete after receiving a sigterm". This is because it's flaky and .kill('SIGTERM') is async.

QA Notes

List any considerations/cases/advice for testing/QA here.

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

• Code generation (copilot but not intellisense)
• Learning or fact checking
• Strategy / design
• Optimisation / refactoring
• Translation / spellchecking / doc gen
• Other
• I have not used AI

You can read more details in our Responsible AI Policy

[doc-han]

@josephjclark
We have some progress here.

1. Tests run on node20 when we're using the swc loader.
2. Hence, we currently have 2 separate CI tests for v18 and v20
3. Most of the issues at the moment are code specific. some things that work in v18 but seem troublesome in v20. Mostly some mocks that need to be updated.

Work in progress!

[josephjclark]

Thank you for the update @doc-han ! Appreciate it

If anything in the core code is seriously breaking please let me know as soon as possible

[doc-han]

Thank you for the update @doc-han ! Appreciate it

If anything in the core code is seriously breaking please let me know as soon as possible

Currently, the only thing not working is mock-fs.
It had some issues due to optimizations introduced to the fs.readfilesync function.

In node 20. mock-fs fails to provide a mock when the mocked fs path is imported instead of read with fs.

This is because. with custom adaptor paths. even tho we mock the adaptor path. the runtime doesn't use fs to read the path. we rather dynamically import he index.js file at that adaptor path.

Been on it for a while now.

[josephjclark]

I see. So we either need to change how the runtime imports modules (not keen on that!) or we need to find a better way to mock modules on the file system?

Or we need to work out a different way to test. We could use the actual file system to load modules, although the mock is very convenient 🤔

Di you have a link for the mock-fs and node 20 issue? Does this fix itself it node 22?

[josephjclark]

Hey @doc-han I think this is going to get complicated, can we divide-and-conquer a bit?

• raise an issue for each isolated node 20 problem (ie node loaders, mock fs)
• create an epic branch to hold all the fixes ready for release
• create a fix branch for each specific issue, targeting the epic branch

This will make code review much easier and help me understand what's going on.

I can help tidy up the issues on Monday, just so long as I have a clear list of the problems

[doc-han]

I see. So we either need to change how the runtime imports modules (not keen on that!) or we need to find a better way to mock modules on the file system?

Or we need to work out a different way to test. We could use the actual file system to load modules, although the mock is very convenient 🤔

Di you have a link for the mock-fs and node 20 issue? Does this fix itself it node 22?

1. readFileSync Issue: readFileSync with 'utf-8' option broken by node v20.5.0 tschaub/mock-fs#377 (fixed via a version bump to 5)

2. don't see a good issue around mock-fs not working with dynamic import in node v20. closest issue for mock-fs@v3Can't require files with mocked filesystem tschaub/mock-fs#130

3. Node has an issue where loader arguments are not passed down to workers. Hence. other loaders like tsx/esm actually work with ava but due to this you'll have to not spawn workers for ava test when using them. ESM loaders cannot be defined via Worker option execArgv in v20 nodejs/node#47747

// ava.config.js
...
  workerThreads: false,
...

---

Our current headache

when mock-fs mocks /modules/

kit/packages/cli/test/commands.test.ts

Lines 60 to 72 in 4376f7b

	mock({
	[expressionPath]: job,
	[statePath]: state,
	[outputPath]: '{}',
	[pnpm]: mock.load(pnpm, {}),
	// enable us to load test modules through the mock
	'/modules/': mock.load(path.resolve('test/__modules__/'), {}),
	'/repo/': mock.load(path.resolve('test/__repo__/'), {}),
	'/monorepo/': mock.load(path.resolve('test/__monorepo__/'), {}),
	//'node_modules': mock.load(path.resolve('node_modules/'), {}),
	[pkgPath]: mock.load(pkgPath),
	...(options.mockfs ?? {}),
	});

we later want to import from this mocked filesystem.

kit/packages/runtime/src/modules/linker.ts

Line 151 in 4376f7b

const result = import(`${prefix}${path}`);

Works in v18 but not in v20

[doc-han]

Hence, our only failing tests are in packages/cli/test/commands.test.ts

[doc-han]

Hey @doc-han I think this is going to get complicated, can we divide-and-conquer a bit?

• raise an issue for each isolated node 20 problem (ie node loaders, mock fs)
• create an epic branch to hold all the fixes ready for release
• create a fix branch for each specific issue, targeting the epic branch

This will make code review much easier and help me understand what's going on.

I can help tidy up the issues on Monday, just so long as I have a clear list of the problems

Yeah we can do that.
My plan was to get things working and then with rebase, rearrange and squash some of the commits into meaningful bits.

[josephjclark]

Ok cool: so let's make this PR solely about #542, and any other node 20 issues we'll handle in different PRs

[doc-han]

@josephjclark
The last commit 48285ba is cherry-pick of #836
will be removed after #836 gets merged and this branch will be rebased.

Resolves our test issues and as you can see. tests are passing on v18 & v20. hurray 🎉 (bed-time)

[josephjclark]

Node has an issue where loader arguments are not passed down to workers. Hence. other loaders like tsx/esm actually work with ava but due to this you'll have to not spawn workers for ava test when using them

Are we saying there's no fix to this? We straight up can't use ava + typescript + workers in node 20+? That would be a huge disappointment - are more people talking about this somewhere?

I'd much prefer to use worker threads, they're way less intensive. Running the tests with child processes makes my whole system burn up. I know, I've tried it!

[doc-han]

Are we saying there's no fix to this? We straight up can't use ava + typescript + workers in node 20+? That would be a huge disappointment - are more people talking about this somewhere?

We currently don't have a problem with that. SWC seems to work fine. That issue was happening with other loaders like tsx.
You'll notice that flag isn't set in ava.config.js

[doc-han]

are more people talking about this somewhere?

will get you references.

[josephjclark]

Ok @doc-han , thanks. So long as swc is loading that's cool, I'm happy to lean into that.

No need for references, happy to take you at your word :) Thanks

[josephjclark]

Starting to see some strange fails on particular node versions. Investigating - not sure how concerned to be yet. Also seeing some strange log output in CI whici I need to get to the bottom of.

Also, I think unit tests have been running wrongly? I think forever? They should be running inside a docker container to use the global openfn command - but they're not, they're running from a local build. This is probably something that needs to be looked at separately

[doc-han]

Starting to see some strange fails on particular node versions.

The uncaught exception error in unit-test is already documented under #844
I've updated the description to elaborate on what's making it error.

[josephjclark]

I've caught a couple of things:

1. Incredibly, the runtime was missing an await on the module loader. It's clearly working fine synchronously (somehow!), but when there's an error it's not getting caught.
2. This is resulting in some tests appearing to pass even though they're throwing errors internally.

I've fixed the runtime, fixed some tests, and skipped some low-value ones

[josephjclark]

Still some flaky tests in docgen which I'll have to keep skipping. But otherwise this is ready go. I'll test the build on the staging server.

I'm feeling pretty confident about this but the last two hours have got me a little worried. So I'l retest with a cool head tomorrow.

[josephjclark]

Thank you so much for this @doc-han 🙏