Hi @henrist,

I really like the change, but I will need some time to consider any possible backwards compatibility implications caused it. I may not be able to properly review this until next week (it's the end of tax season over here which limits my extra time), but based on a quick scroll through I think it looks great overall. Backwards compatibility and potential breakage is my main concern and I'm grateful that you seem to have kept that in mind. For now, we are not ready to fully remove ts-node but that may be a possibility in a future major version.

Do note that while this PR may add full ESM support on the CLI side, our runtimes do not yet support ESM due to complexities of the current architecture, so unfortunately this PR does not fully enable ESM on the entire Checkly platform. However, we have a project in progress to allow ESM to work on our runners, so ESM everywhere may soon be a possibility too. For now though it's a limitation that you have to keep in mind.

Thanks a lot!

@sorccu Thanks for your quick feedback! Looking forward to your review. I have put some effort into avoiding this to become a breaking change. I've tested it an original template project, new template project and on a couple of our repos. But useful to have someone else double check.

Great to hear that you're working on ESM support on the runner side as well, although due to the limitation of dependencies and the fact that it parses module syntax just fine, this haven't been a real problem for us. I did use the term "config" in the PR title in an attempt to make it clear that this is for config of checks, not the check scripts/specs themself.

Would it be possible to trigger the canary/experimental release for this PR?

I would be happy to trigger a build, but unfortunately that will also have to wait till next week as our workflows do not currently give access to secrets for PRs from forks, which means that both integration tests and the build would fail. I will massage the workflows a little next week to make that work.

Unfortunately I was extra busy last week and did not have bandwidth to test your PR, but I will handle this within the next 2 days. Super excited about this feature.

Unfortunately I had to come up with a temporary janky solution to run the workflows by creating a non-fork PR with your changes in #1048, but since the commit hash is the same, the test results can be seen in this original PR too. I am happy to keep merging any changes to the build PR and make any other changes as needed.

An experimental build is also available: [email protected]

However, as can be seen from the result of the test run, there seems to be some sort of an incompatibility. I will look into this shortly.

That's great! I'll test out the experimental build on my end tomorrow or Thursday, and also see if I can understand the reason for the failure. I have 30+ repos here with Checkly CLI, however they are pretty homogeneous.

npm run test runs fine here on same Node.js version (but I'm on MacOS). Could it be platform-specific problem (the failures were on Windows)? I see the Linux job never ran. Would be nice to set strategy.fail-fast: false on the job.

Indeed looks like a platform problem as it runs fine on ubuntu-latest. I wonder if this is a Jest issue and a caching bug in Jiti.

Running it with debug mode shows that under Windows some additional file are being transpiled. It seems under Linux those same files exists in Node.js module cache, but on Windows I think it becomes a mix of path styles so the cache is not used.

Even if we set the --experimental-vm-modules option it won't work, since it would load a fresh constructs file so the global state in Session will be different/not so global.

This problem is limited to testing here though, as regular users won't have this behavior since the Checkly CLI code is compiled to JS. A bit sure what would be the best fix. I'll anyways see if I can create a repro for the potential caching bug.