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

8.1.0 breaks named imports when using ESM #8320

Closed
3 of 15 tasks
BenSjoberg opened this issue Oct 13, 2021 · 12 comments
Closed
3 of 15 tasks

8.1.0 breaks named imports when using ESM #8320

BenSjoberg opened this issue Oct 13, 2021 · 12 comments
Labels
needs triage This issue has not been looked into

Comments

@BenSjoberg
Copy link

BenSjoberg commented Oct 13, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

8.1.0 must seems to have changed something with exports, which affects projects using pure ESM. After updating, my application fails to launch with the following message:

import { Controller, Get, Header } from '@nestjs/common';
         ^^^^^^^^^^
SyntaxError: Named export 'Controller' not found. The requested module '@nestjs/common' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

Downgrading to 8.0.11 resolves the issue. A workaround without downgrading is to do something like:

import nestcommon from '@nestjs/common';
const import { Controller, Get, Header } = nestcommon;

But this is pretty clumsy.

Our project transpiles TypeScript to ESM, instead of to CommonJS, since some of our dependencies have moved to pure ESM without a CommonJS fallback.

Minimum reproduction code

https://github.com/BenSjoberg/nest-esm-import-issue-example

Steps to reproduce

  • Install @nestjs/common 8.0.11
  • Create a file called test.mjs with the content import { Controller } from '@nestjs/common';
    • Alternatively, named the file test.js and set "type": "module" in package.json
  • Run node test.mjs - it should work.
  • Upgrade @nestjs/common to 8.1.0, it will fail with the error I described.

The sample repo I linked shows the issue in a working nest.js project. The initial commit (bfcd0c99) works because it's pinned to Nest 8.0.x, the latest commit doesn't work because I updated to 8.1.0.

Expected behavior

Named imports should work from pure ESM as they did in 8.0.x.

Package

  • 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

No response

Packages versions

  • @nestjs/cli: 8.1.2
  • @nestjs/common: 8.1.0
  • @nestjs/core: 8.1.0
  • @nestjs/platform-express: 8.1.0
  • @nestjs/schematics: 8.0.3
  • @nestjs/testing: 8.1.0

Node.js version

16.11.1

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@BenSjoberg BenSjoberg added the needs triage This issue has not been looked into label Oct 13, 2021
@BenSjoberg
Copy link
Author

I did a little more digging, and it seems like this is related to the upgrade to TypeScript 4.4. TypeScript changed how function calls work in emitted code in some circumstances, which is causing named exports to stop working in ESM. This likely affects more than just Nest. microsoft/TypeScript#45813

Would it be possible to downgrade TS to 4.3 until they're able to fix this?

@kamilmysliwiec
Copy link
Member

Thanks for investigating this issue! Temporarily reverted & fixed in 8.1.1

@BenSjoberg
Copy link
Author

Thanks so much! 😃

@vadistic
Copy link

vadistic commented Nov 2, 2021

@kamilmysliwiec FYI same thing is still broken for @nestjs/graphql nestjs/graphql#1843

@BenSjoberg
Copy link
Author

This appears to have broken again in 8.3.0 due to 329b083.

Would it be possible keep TypeScript at 4.3.x until 4.6 is released? Based on my testing in the dev release of TypeScript, the problem that causes this will be fixed in 4.6.

@sionzee
Copy link

sionzee commented Feb 16, 2022

@BenSjoberg I think this deserves a new issue otherwise it won't get fixed.

@kamilmysliwiec
Copy link
Member

Fixed in 8.3.1

@huan
Copy link

huan commented Feb 23, 2022

I run into this issue today with @nestjs/[email protected]:

[email protected]:~/git/wechaty/friday/src/core/statuspage/sagas [19:31:42] tty:[3] jobs:[0]
└ {nestjs} $ ./statuspage.saga.spec.ts 
file:///home/huan/git/wechaty/friday/src/wechaty-repository/wechaty.repository.ts:2
import { Injectable, OnModuleDestroy, OnModuleInit } from '@nestjs/common';
                     ^^^^^^^^^^^^^^^
SyntaxError: Named export 'OnModuleDestroy' not found. The requested module '@nestjs/common' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@nestjs/common';
const { Injectable, OnModuleDestroy, OnModuleInit } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:181:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

$ npm ls @nestjs/common
[email protected] /home/huan/git/wechaty/friday
├── @nestjs/[email protected]
├─┬ @nestjs/[email protected]
│ └── @nestjs/[email protected] deduped
├─┬ @nestjs/[email protected]
│ └── @nestjs/[email protected] deduped
├─┬ @nestjs/[email protected]
│ └── @nestjs/[email protected] deduped
├─┬ @nestjs/[email protected]
│ └── @nestjs/[email protected] deduped
├─┬ @nestjs/[email protected]
│ └── @nestjs/[email protected] deduped
└─┬ @nestjs/[email protected]
  └── @nestjs/[email protected] deduped

┌ [email protected]:~/git/wechaty/friday/src/core/statuspage/sagas [19:33:40] tty:[3] jobs:[0]
└ {nestjs} $ npm ls typescript
[email protected] /home/huan/git/wechaty/friday
├─┬ @chatie/[email protected]
│ └─┬ @typescript-eslint/[email protected]
│   └─┬ [email protected]
│     └── [email protected] deduped
├─┬ @chatie/[email protected]
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ ├─┬ [email protected]
│ │ └─┬ [email protected]
│ │   └── [email protected] deduped
│ └── [email protected]
├─┬ @nestjs/[email protected]
│ ├─┬ [email protected]
│ │ └── [email protected] deduped
│ └── [email protected] deduped
└─┬ @nestjs/[email protected]
  └── [email protected] deduped

┌ [email protected]:~/git/wechaty/friday/src/core/statuspage/sagas [19:37:40] tty:[3] jobs:[0]
└ {nestjs} $ node --version
v16.13.2

Reproduce repo:

https://github.com/wechaty/friday/tree/25c04016497a7f3af44e6563b5c88e0f8a8ffe25

@sionzee
Copy link

sionzee commented Feb 23, 2022

@huan
The bug was again fixed in 8.3.1 and it works without issues.

That must be an issue on your side, if you generate a new project, you will notice it works out of the box.

Looks like you are using SWC, have you make a right configuration for SWC to be running in es mode? The default tool for test is ts-node at nestjs, in case of swc you must enable decorators + proper settings for latest ES.

@huan
Copy link

huan commented Feb 23, 2022

@sionzee Thanks for the quick reply!

I'm working on it and trying to figure out the problem right now.

My case is very strange: all the code works without any problem this afternoon; after I do an rm -fr node_modules/ && npm i, then this error message jumped out.

For example, you can see that version 1.13.8 has passed the CI tests, which means it has no this problem at that time:

image

The PR is at:

My SWC works in the past days. I guess my problem is related to the node_modules file change.

I will update here when I got any updates.

@huan
Copy link

huan commented Feb 26, 2022

Update

it seems that @sionzee is right, my issue is related to swc.

I have created an issue on my repo for continuing tracking my problem.

Thank you very much!

@Toilal
Copy link

Toilal commented Feb 26, 2022

I hit this issue, and I can confirm it's fixed by upgrading.

huan added a commit to wechaty/friday that referenced this issue Feb 27, 2022
* init code

* add gitter & oicq bot

* add config class

* add oa builder

* add whatsapp builder

* add wxwork

* add wechaty bots nestjs module

* mix all files

* code clean

* code clean

* add web root to controller

* restructure folders

* restructure folders

* code clean & import / export CQRS

* code clean & import / export CQRS

* restructure settings folder

* clean database deprecated import

* clean database deprecated import

* add status counting service & queries

* add commands & settings

* restruct setting folder

* refactoring code

* finish status page saga & refactoring

* code clean

* rename

* finish events for message

* rename oicq -> qq

* code clean

* re-struct codes

* re-struct codes

* rename mo/mt

* code clean

* add mermaid.js diagram dchart

* support awesome font brand icons

* code clean

* finish room events saga handlers

* re-structure folder

* clean settings

* clean

* clean

* finish query for sync domain

* add sync room commands

* clean

* clean code for command handlers

* use Logger instead of Brolog

* code clean

* add wechat rooms

* finish code & pass linting

* finish code & pass linting

* finalizing the code

* 1.13.1

* prepare wechaty entities

* re-draw mermaid diagram

* code clean

* code clean

* restruct folders

* add e2e unit tests with injection

* clean

* clean

* 1.13.2

* use injection for plugin settings

* 1.13.3

* remove deprecated settings

* 1.13.4

* restructure folders & files

* add WechatyEventsModule

* restructure folders

* clean folder paths

* 1.13.5

* debuging inject prorblems

* 1.13.6

* fix all injections!

* fix

* clean

* 1.13.7

* fix injections

* code clean & unit test passed!

* fix unit tests

* upgrade whatsapp

* clean

* 1.13.8

* code clean

* 1.13.9

* fix io-client ws connecting close error

* code clean

* 1.13.10

* code clean

* 1.13.11

* disable statuspage temporary for debugging

* 1.13.12

* restruct folders

* 1.13.13

* fix plugin install twice bug

* add mo-mt-saga to generate commands from event

* code clean

* code clean

* clean

* fix missed handlers for commands bug

* 1.13.14

* sync server data

* use fetch for statuspage api

* try / catch in command handlers

* add unit tests for saga/rxjs

* 1.13.15

* upgrade deps, strange problem (nestjs/nest#8320)

* 1.13.16

* 1.13.17

* workaround swc (#114)

* add marble unit tests for rxjs saga

* 1.13.18

* catch io client rejections in e2e test

* 1.13.19

* fix bugs

* 1.13.20

* try/catch rejections

* 1.13.21

* fix chatops web api

* 1.13.22

* add marble tests

* 1.13.23
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

6 participants