refactor(common): Prevent JavaScript being wrapped in eval

[Borewit]

Replace wrapping dynamic import of ESM module in eval with load-esm.

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

Wraps the dynamic import in an eval function, which is not the best workaround to avoid the TypeScript compiler breaks the dynamic import.

More information: https://stackoverflow.com/a/79265806/28701779

What is the new behavior?

Is uses load-esm instead, which is, just like eval, using a simple trick to keep the dynamic import outside the scope of the TypeScript compiler, yet not by wrapping the entire ESM module in an eval method.

Not black and white science, wrapping code in eval this case the import and the entire ESM module, is not such a great idea:

• https://stackoverflow.com/a/198031/28701779

Does this PR introduce a breaking change?

• Yes
• No

Other information

Related to discussion #14970 (comment)

[coveralls]

Pull Request Test Coverage Report for Build c1fde52d-d1f3-4297-b188-e9e2580fab05

Details

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 89.321%

---

Totals
Change from base Build 78476dc7-1b28-4533-b625-21f927bc909f:	0.001%
Covered Lines:	7160
Relevant Lines:	8016

---

💛 - Coveralls

[Chathula]

do we need load-esm here. can't we use just inline import ?

[Borewit]

do we need load-esm here. can't we use just inline import ?

In JavaScript, yes, no problem. But in TypeScript that does not work, as the TypeScript compiler converts the dynamic import to require. The require will fail to load the ESM module, is it in principal designed to load ESM modules.

The exception that this does work if you use a recent Node.js engine 22, as in that engine the require is able to load the ESM module, which compensates for the incorrect conversion op the TypeScript compiler.

[fantasywind]

load-esm dependency seems break nextjs monorepo project with nx, used typescript alias path

[Borewit]

I need to adjust my reply provider here: #14974 (comment)

I just found an issue regarding this arbitrary dynamic import to require conversion: microsoft/TypeScript#54111

So it seems that this is bound to the Compiler options module and moduleResolution.

One way of preserving the dynamic import is:

{
  "compilerOptions": {
    "module": "node16",
    "moduleResolution": "node16",
  }
}

So to use node16 instead of commonjs. Which seems to be a more modern version of commonjs. So that way we could eliminate the load-esm usage / dependency.

Shall I create a PR?

Created a PR, for this alternative solution: #14976

[fz6m]

You could try creating an extra package.json as a sub-package to inline load-esm :

 - load-esm
   - package.json
   - index.js
 - file.ts

// load-esm/pacakge.json
{
  "type": "commonjs"
}

// file.ts
import { loadEsm } from './load-esm'

// use `loadEsm`

or use a precompiled solution like Next.js with ncc to compile file-type into CJS and inline it into the project.

this might help reduce dependencies and lower the risk of supply chain attacks ( in the long term, we could precompile all dependencies ).

[benscobie]

load-esm dependency seems break nextjs monorepo project with nx, used typescript alias path

We also use nextjs in a monorepo, though with turborepo. We have a separate project to share our DTOs between the frontend and backend using nestjs-zod. That unfortunately means some of the nestjs dependencies get brought into the UI which has thrown up this warning after this change:

⚠ ../node_modules/load-esm/index.js
Critical dependency: the request of a dependency is an expression

Import trace for requested module:
../node_modules/load-esm/index.js
../node_modules/@nestjs/common/pipes/file/file-type.validator.js
../node_modules/@nestjs/common/pipes/file/index.js
../node_modules/@nestjs/common/pipes/index.js
../node_modules/@nestjs/common/index.js
../node_modules/nestjs-zod/dist/index.js
../packages/contracts/dist/settings/logs/logSetting.dto.js
../packages/contracts/dist/settings/logs/index.js
../packages/contracts/dist/index.js
./src/contexts/taskstatus-context.tsx

EDIT: I swapped our import of nestjs-zod to nestjs-zod/dto which doesn't require the nestjs dependencies, that resolved things for us.