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

--disallow-code-generation-from-strings breaks nest's named ESM exports #10846

Closed
6 of 15 tasks
ephys opened this issue Jan 10, 2023 · 2 comments
Closed
6 of 15 tasks

--disallow-code-generation-from-strings breaks nest's named ESM exports #10846

ephys opened this issue Jan 10, 2023 · 2 comments
Labels
needs triage This issue has not been looked into

Comments

@ephys
Copy link

ephys commented Jan 10, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

If you run node with the --disallow-code-generation-from-strings option & use native ESM imports, some named ESM imports break.

Minimum reproduction code

https://github.com/ephys/nest-esm-issue

Steps to reproduce

  1. npm ci
  2. node index.mjs # should run without issues
  3. node --disallow-code-generation-from-strings index.mjs # should break: named import not found

Expected behavior

For named exports to be available even when --disallow-code-generation-from-strings is used

Package

I've only tested the first 4 packages of this list, only @nestjs/core succeeded (but it's possible one of the imports is failing. This issue is likely relevant to all packages):

  • I don't know. Or some 3rd-party package
  • @nestjs/common
  • @nestjs/core
  • @nestjs/microservices
  • @nestjs/platform-express
  • @nestjs/platform-fastify
  • @nestjs/platform-socket.io
  • @nestjs/platform-ws
  • @nestjs/testing
  • @nestjs/websockets
  • Other (see below)

Other package

No response

NestJS version

9.2.1

Packages versions

"@nestjs/core": "^9.2.1",
"@nestjs/common": "^9.2.1",
"@nestjs/microservices": "^9.2.1",
"@nestjs/platform-express": "^9.2.1",

Node.js version

18.13.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

I think this can be fixed by including an ESM export map file, like I've done in other projects here: https://github.com/sequelize/sequelize/blob/main/src/decorators/legacy/index.mjs (+ test to ensure it stays up to date)

@ephys ephys added the needs triage This issue has not been looked into label Jan 10, 2023
@jmcdo29
Copy link
Member

jmcdo29 commented Jan 10, 2023

We haven't set up ESM support yet, so anything that is working is not by design and anything that isn't working is not a bug. When we make Nest compatible with ESM this will be resolved. In the meantime, I followed the errors from the output and wrote the import { Logger } ... as

import pkg from '@nestjs/common'
const { Logger } = pkg

and your code worked. Annoying, sure, but it works

@jmcdo29 jmcdo29 closed this as completed Jan 10, 2023
@4-1-8
Copy link

4-1-8 commented Jan 11, 2023

Wouldn't it be useful to keep it open so it can be added as tests later when Nest will support ESM modules?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

3 participants