Rethrow error if not ESM import error

[karlhorky]

Hey @lukeed, hope you're good!

Today when running ley new we received an error with ERR_REQUIRE_ESM:

• ley.config.mjs which had a JS error in it
• error swallowed by the try/catch in load() from utils.js
• require() ran on the ley.config.mjs file, which results in ERR_REQUIRE_ESM instead, which was confusing

This PR instead rethrows the error if it's not an error related to importing the ESM file (inspired by similar code in tsm)

---

In the meantime, while this PR is waiting on a review, we've published our fork here:

• @upleveled/ley on npm

[karlhorky]

Ah, I think ERR_REQUIRE_ESM from my new code is the wrong error code - I guess it's the opposite error code from what we want...

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.54%. Comparing base (0cf486e) to head (c218b21).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
+ Coverage   48.82%   49.54%   +0.71%     
==========================================
  Files           2        2              
  Lines         213      220       +7     
==========================================
+ Hits          104      109       +5     
- Misses        109      111       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[karlhorky]

Ok, was a bit more complicated than matching a single error code, but it seems to be working now, and not too much code:

// Rethrow error if it is not any of:
// - not found error
// - old Node.js compatibility error
if (e && e.message && !e.code === 'ERR_MODULE_NOT_FOUND' &&
  !['Not supported', 'require(...).pathToFileURL is not a function'].includes(e.message)
) throw e;

[karlhorky]

@lukeed friendly ping, have any thoughts on this one?

[karlhorky]

@lukeed Ah just found a logic error in my implementation, here's the fixed version from the last commits:

		// Rethrow error if it is not any of:
		// - not found error
		// - old Node.js compatibility error
		if (e && e.message &&
-			!e.code === 'ERR_MODULE_NOT_FOUND' &&
+			e.code !== 'ERR_MODULE_NOT_FOUND' &&
			!['Not supported', 'require(...).pathToFileURL is not a function'].includes(e.message)
		) throw e;

[karlhorky]

Edit: outdated - see my comment below: I worked around this with an additional check for a /migrations/*.ts path in the error message

Show outdated comment content

❗️(large) Caveat: this disables support for TypeScript migration files when using --require tsm or --require tsx/cjs:

$ pnpm migrate up

> next-js-example-winter-2024-atvie@0.1.0 migrate /Users/k/projects/next-js-example-winter-2024-atvie
> ley --require tsx/cjs "up"

[ley] Loading configuration

[ley] An error occurred:

      TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /Users/k/projects/next-js-example-winter-2024-atvie/migrations/00000-createTableAnimals.ts
        at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
        at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
        at defaultLoad (node:internal/modules/esm/load:143:22)
        at async ModuleLoader.load (node:internal/modules/esm/loader:409:7)
        at async ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:45)
        at async link (node:internal/modules/esm/module_job:76:21)
      code: ERR_UNKNOWN_FILE_EXTENSION

 ELIFECYCLE  Command failed with exit code 1.

Workaround

Instead of ley --require tsm, use NODE_OPTIONS='--import tsx' ley (on non-Windows machines), as proposed here:

• Document --import tsx as alternative to tsm #37

[karlhorky]

Since Windows does not support npm scripts setting environment variables NODE_OPTIONS in the same way as macOS and Linux, I'll add an additional condition to handle this ERR_UNKNOWN_FILE_EXTENSION error code instead.

[karlhorky]

@lukeed we revisited this today because a student was having problems with the condition still swallowing the error on the following import:

import { setEnvironmentVariables } from 'util/config';

This led to the following confusing ESM error (we use "type": "module" in our projects and the fallback require() in Ley cannot require() an ES Module):

$ pnpm ley new createUsers.ts --esm
C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\node_modules\@upleveled\ley\lib\util.js:71
                return require(id);
         ^

Error [ERR_REQUIRE_ESM]: require() of ES Module C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\ley.config.js from C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\node_modules\@upleveled\ley\lib\util.js not supported.
Instead change the require of ley.config.js in C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\node_modules\@upleveled\ley\lib\util.js to a dynamic import() which is available in all CommonJS modules.
    at exports.load (C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\node_modules\@upleveled\ley\lib\util.js:71:10)
    at async C:\Users\HUAWEI\projects\books-inspo-app\books-inspo\node_modules\@upleveled\ley\bin.js:11:13 {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v20.11.1

But the real error was actually the missing .js in the fully-specified import path - should have been this:

import { setEnvironmentVariables } from 'util/config.js';

Without this error swallowing behavior of Ley, the error message would have been the following, which is much more helpful:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/lukas/Documents/UpLeveled/2024winter/julia/books-inspo-app/books-inspo/util/config' imported from /Users/lukas/Documents/UpLeveled/2024winter/julia/books-inspo-app/books-inspo/ley.config.js
Did you mean to import ../util/config.js?
    at finalizeResolution (node:internal/modules/esm/resolve:264:11)
    at moduleResolve (node:internal/modules/esm/resolve:917:10)
    at defaultResolve (node:internal/modules/esm/resolve:1130:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:396:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:365:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:85:39)
    at link (node:internal/modules/esm/module_job:84:36) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///Users/lukas/Documents/UpLeveled/2024winter/julia/books-inspo-app/books-inspo/util/config'
}

For this reason, I simplified the rethrow condition (and fixed the test) in c218b21

If you would have an opportunity to review this PR and offer some feedback, it would be great!