Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: override ts-node module behaviour [gh-768] #804

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

clample
Copy link
Collaborator

@clample clample commented Jul 27, 2023

I hereby confirm that I followed the code guidelines found at engineering guidelines

Affected Components

  • CLI
  • Create CLI
  • Test
  • Docs
  • Examples
  • Other

Notes for the Reviewer

Resolves #768

The bug

Currently the CLI is unable to support TypeScript checkly.config.ts and **.check.ts files in projects that are configured as ECMAScript Module projects. This can be reproduced by setting "type": "module" in package.json for the boilerplate project:

$ npx checkly deploy
Parsing your project... !
    Error: Error loading file /Users/clample/projects/test/amber-snake/checkly.config.ts
    Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/clample/projects/test/amber-snake/checkly.config.ts
    require() of ES modules is not supported.
    require() of /Users/clample/projects/test/amber-snake/checkly.config.ts from /Users/clample/projects/test/amber-snake/node_modules/checkly/dist/services/util.js is an ES module file as it is a .ts file whose nearest parent package.json contains "type": "module" which defines all .ts files in that package scope
    as ES modules.
    Instead change the requiring code to use import(), or remove "type": "module" from /Users/clample/projects/test/amber-snake/package.json.

        at createErrRequireEsm (/Users/clample/projects/test/amber-snake/node_modules/ts-node/dist-raw/node-internal-errors.js:46:15)
        at assertScriptCanLoadAsCJSImpl (/Users/clample/projects/test/amber-snake/node_modules/ts-node/dist-raw/node-internal-modules-cjs-loader.js:584:11)
        at Object.require.extensions.<computed> [as .ts] (/Users/clample/projects/test/amber-snake/node_modules/ts-node/src/index.ts:1610:5)
        at Module.load (node:internal/modules/cjs/loader:1004:32)
        at Function.Module._load (node:internal/modules/cjs/loader:839:12)
        at Module.require (node:internal/modules/cjs/loader:1028:19)
        at require (node:internal/modules/cjs/helpers:102:18)
        at /Users/clample/projects/test/amber-snake/node_modules/checkly/src/services/util.ts:69:54
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at async loadTsFile (/Users/clample/projects/test/amber-snake/node_modules/checkly/src/services/util.ts:69:33)

The issue occurs when importing the project files via ts-node. The ts-node documentation has extensive notes on this: ts-node CommonJS vs native ECMAScript modules and ts-node Module type overrides.

The Fix

The Checkly CLI is currently compiled to CommonJS and the await import in loadTsFile is compiled to a require:

async function loadTsFile(filepath) {
    try {
        const tsCompiler = await getTsCompiler();
        tsCompiler.enabled(true);
        let { default: exported } = await (_a = filepath, Promise.resolve().then(() => require(_a)));
        if (exported instanceof Function) {
            exported = await exported();
        }
        tsCompiler.enabled(false); // Re-disable the TS compiler
        return exported;
    }
    catch (err) {
        throw new Error(`Error loading file ${filepath}\n${err.stack}`);
    }
}

One option could be to leave the MaC project code as an ECMAScript Module, and import it using a similar trick we use for .mjs support (here. This would be difficult, though, with ts-node. We would need to use ts-node's Native ECMAScript Modules Support, and this requires passing the --loader flag to node.

Instead, this PR configures ts-node to ignore the project's package.json and compile the code to CommonJS. This is described in ts-node Module Type Overrides. This let's us continue to require the project's checkly.config.ts and *.check.ts files.

One downside with this approach may be with projects importing other dependencies that are pure ESM packages. In this case, I think that they will run into ERR_REQUIRE_ESM.

@clample clample added the build Issue regarding building and packaging label Jul 27, 2023
@github-actions
Copy link

🎉 Experimental release successfully published on npm

npm install checkly@0.0.0-pr.804.7a3174d

@clample clample requested a review from umutuzgur July 27, 2023 10:30
Copy link
Member

@umutuzgur umutuzgur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@clample clample merged commit 46470cd into main Jul 27, 2023
4 checks passed
@clample clample deleted the chrislample/type-module-bug branch July 27, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issue regarding building and packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module
2 participants