denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.71k stars 5.34k forks source link

Support NestJS (test with jest) #20619

Open birkskyum opened 1 year ago

birkskyum commented 1 year ago

Repro:

Expected:

$ jest
 PASS  src/app.controller.spec.ts
  AppController
    getHello
      ✓ should return "Hello World!" (1 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.068 s
Ran all test suites.

Actual

➜ deno --unstable task test
Warning Currently only basic package.json `scripts` are supported. Programs like `rimraf` or `cross-env` will not work correctly. This will be fixed in an upcoming release.
Task test jest
Unstable API 'Deno.connect'. The --unstable flag must be provided.
birkskyum commented 1 year ago

And trying to bypass this using deno --unstable run -A npm:jest yields:

➜ deno --unstable run -A npm:jest
watchman warning:  Recrawled this watch 5 times, most recently because:
MustScanSubDirs UserDroppedTo resolve, please review the information on
https://facebook.github.io/watchman/docs/troubleshooting.html#recrawl
To clear this warning, run:
`watchman watch-del '/Users/admin/repos/deno-kitchensink/nest-project' ; watchman watch-project '/Users/admin/repos/deno-kitchensink/nest-project'`

TypeError: this._child.send is not a function
    at ChildProcessWorker.initialize (file:///Users/admin/repos/deno-kitchensink/nest-project/node_modules/.deno/jest-worker@29.7.0/node_modules/jest-worker/build/workers/ChildProcessWorker.js:169:17)
bartlomieju commented 1 year ago

This is blocked by IPC support in node:child_process module. I will be looking into that.

birkskyum commented 11 months ago

With deno 1.38, I can't get past this message:

➜ deno task --unstable test           
Task test jest
Unstable API 'Deno.connect'. The --unstable flag must be provided.
bartlomieju commented 11 months ago

@birkskyum out of curiosity - does it get any further if you create deno.json with:

{
  "unstable": ["net"]
}

?

birkskyum commented 11 months ago

Yes, with the deno.json I see:

➜ deno task test
Task test jest
TypeError: this._child.send is not a function
    at ChildProcessWorker.initialize (file:///Users/admin/repos/nest-test/project/node_modules/.deno/jest-worker@29.7.0/node_modules/jest-worker/build/workers/ChildProcessWorker.js:169:17)
bartlomieju commented 11 months ago

So we're back at the missing IPC support - good news is: we figured out a way forward to provide this mechanism and I'll be working on PoC for Unix systems this week.

malthe commented 10 months ago

You can run tests in-band using -i which doesn't require a child-process worker.

$ deno --unstable run -A npm:jest -i

But Jest relies on v8.serialize which is not implemented by Deno. The compatibility notes mention that:

[these APIs] could be polyfilled, but due inherent lack of format stability between the V8 versions, the Deno team is considering requiring a special flag to use them.

birkskyum commented 10 months ago

with deno 1.39 i get:

➜ deno task test 
Task test jest
Error: Not implemented: v8.serialize
    at notImplemented (ext:deno_node/_utils.ts:9:9)
    at serialize (node:v8:59:3)
    at HasteMap._persist (file:///Users/admin/repos/deno-kitchensink/project/node_modules/.deno/jest-haste-map@29.7.0/node_modules/jest-haste-map/build/index.js:716:26)
    at file:///Users/admin/repos/deno-kitchensink/project/node_modules/.deno/jest-haste-map@29.7.0/node_modules/jest-haste-map/build/index.js:380:16
    at Object.runMicrotasks (ext:core/01_core.js:820:30)
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:53:10)
    at runNextTicks (ext:deno_node/_next_tick.ts:71:3)
    at eventLoopTick (ext:core/01_core.js:188:21)
    at async file:///Users/admin/repos/deno-kitchensink/project/node_modules/.deno/@jest+core@29.7.0/node_modules/@jest/core/build/cli/index.js:268:9
    at async Promise.all (index 0)
GerbenRampaart commented 5 months ago

I tried the same thing. Deno 1.43.1.

Cloned the typescript-starter like @birkskyum

2024-05-03 17 42 33

TypeError: Class constructor Process cannot be invoked without 'new'

GerbenRampaart commented 5 months ago

Since I'm invested in seeing NestJs properly supported by Deno or Bun (we have many apis in our corporation developed in nest) I did some additional testing.

So I created a brand new project with the nest cli and made the following changes to get as close to a runnable project as possible:

Running deno test yields these results:

2024-05-06 12 28 37

Please verify these results by cloning: https://github.com/GerbenRampaart/deno-nestjs-vanilla

This error is decorator related right?

GerbenRampaart commented 5 months ago

Ok, a bit weird. If I add "emitDecoratorMetadata": true to deno.json (which is not documented in the deno.json schema), it does work.

2024-05-06 12 50 57

2024-05-06 12 51 15

GerbenRampaart commented 5 months ago

@birkskyum @bartlomieju

2024-05-06 16 19 20

Integration test works, unit test works. Check the repo.

Further changes from vanilla nestjs project:

I think we can call this issue closed. Deno happily supports nestjs from the looks of it.

birkskyum commented 5 months ago

Did you find a solution to the "TypeError: Class constructor Process cannot be invoked without 'new'" issue?

https://github.com/denoland/deno/issues/20619#issuecomment-2093269631

GerbenRampaart commented 5 months ago

Did you find a solution to the "TypeError: Class constructor Process cannot be invoked without 'new'" issue?

#20619 (comment)

I didn't continue with the cloned variant of that typescript-starter of nestjs you started this issue with.

I figured that the @nest/cli also scaffolds a new vanilla nest project and with the most modern approach, the typescript-starter was initially committed a couple of years ago.

birkskyum commented 5 months ago

I tried making an new project with the cli approach instead just now, and I got the same TypeError: Class constructor Process cannot be invoked without 'new' error when running test.

GerbenRampaart commented 5 months ago

I tried making an new project with the cli approach instead just now, and I got the same TypeError: Class constructor Process cannot be invoked without 'new' error when running test.

Do you mind cloning my repo: https://github.com/GerbenRampaart/deno-nestjs-vanilla and share the output you get?

deno task test or deno test

and

deno task test:e2e

birkskyum commented 5 months ago

cloning my repo: https://github.com/GerbenRampaart/deno-nestjs-vanilla

These tests passed, both for test:e2e and test

So deno can run nestjs projects tailored specifically to deno, but it would be nice to be able to run existing projects with node compat.

GerbenRampaart commented 5 months ago

I think this is a good starting point for me @birkskyum, maybe your explicit goal was to make sure Deno executed the jest code explicitly, and not use Deno.test.

But honestly I'm not going to chase that dragon, the Deno.test looks very clean and the core issue for me was that nestjs was supported as platform/framework, with working dependency injection and so on, which it does.

birkskyum commented 5 months ago

That's reasonable - my focus was indeed on making initial adoption of deno frictionless, but using/migrating to the deno primitives like you do is ofc. ideal.

Is there an official starter for deno, like the one you shared?

GerbenRampaart commented 5 months ago

Is there an official starter for deno, like the one you shared?

If there is, I haven't found it. The nestjs people can be, with respect, ever so slightly stringent. If you open a Deno issue there it's closed before you can say 'ecmascript modu...'. I don't really blame them, they get mountains of questions and issues not caused by nestjs.

Regardless, I guess we're free to publish a Deno+Nest demo project for interested people to see anywhere, including somewhere in the Deno recipes in their docs.

GerbenRampaart commented 5 months ago

For reference, I did just stumble across this issue #20595 who's repo (https://github.com/mfulton26/deno-nest) draws some of the same conclusions I did. And they re-wrote the test using a BDD approach (like jest is):

https://deno.land/std@0.224.0/testing/bdd.ts

While I haven't pulled that repo and ran the test, I guess this is also an approach.

It still involves somewhat altering the test a bit (the import), but less so than Deno.test.

Maybe that's closer to your out-of-the-box support for jest-like testing @birkskyum .

bartlomieju commented 5 months ago

Thanks for the updates @GerbenRampaart @birkskyum, happy to hear that NestJS is now somewhat usable with Deno!

I tried making an new project with the cli approach instead just now, and I got the same TypeError: Class constructor Process cannot be invoked without 'new' error when running test.

This error is gonna really painful to fix 😬 the problem seems to be that we use class Process and the code of interest is using ES5 syntax to instantiate, eg.:

if (!(this instanceof Process)) {
  Process.call(...)
}

I wonder if we should try to fix it upstream 🤔

birkskyum commented 5 months ago

I think it makes sense to fix jest-worker upstream, because it's so widely used. It's somewhat similar to this issue

birkskyum commented 1 month ago

Alright, I took the Deno 2 rc.2 for a spin, and could run this:

git clone https://github.com/nestjs/typescript-starter.git project
cd project
deno i
deno task test
deno task build

But what fails still is:

deno task test:e2e

➜ deno task test:e2e                
Task test:e2e jest --config ./test/jest-e2e.json
 FAIL  test/app.e2e-spec.ts
  AppController (e2e)
    ✕ / (GET) (7 ms)

  ● AppController (e2e) › / (GET)

    TypeError: error sending request for url (http://127.0.0.1:0/): client error (Connect): tcp connect error: Can't assign requested address (os error 49): Can't assign requested address (os error 49)

      at Test.end [as _end] (../node_modules/.deno/superagent@8.1.2/node_modules/superagent/src/node/index.js:1274:9)
      at Test._end [as end] (../node_modules/.deno/superagent@8.1.2/node_modules/superagent/src/node/index.js:1000:8)
      at Test.end (../node_modules/.deno/supertest@6.3.4/node_modules/supertest/lib/test.js:118:11)
      at end (../node_modules/.deno/superagent@8.1.2/node_modules/superagent/src/request-base.js:285:12)
          at undefined
      at Test.Object.<anonymous>.RequestBase.then (../node_modules/.deno/superagent@8.1.2/node_modules/superagent/src/request-base.js:267:31)
          at undefined
      at Object.runMicrotasks (../ext:core/01_core.js:672:26)
      at processTicksAndRejections (../ext:deno_node/_next_tick.ts:57:10)
      at runNextTicks (../ext:deno_node/_next_tick.ts:75:3)
      at eventLoopTick (../ext:core/01_core.js:182:21)