eugef / node-mocks-http

Mock 'http' objects for testing Express,js, Next.js and Koa routing functions
Other
755 stars 134 forks source link

Fix nodejs typing #281

Closed mhaligowski closed 10 months ago

mhaligowski commented 10 months ago

Current types definition for the node-mocks-http library extend the HTTP requests and responses from express. As a result, HTTP request/response implementations from other frameworks (e.g. NextApiRequest) are type-incompatible with node-mocks-http.

This changes the generic class of the mock objects from the Request/Response from express library to the NodeJS IncomingMessage/OutgoingMessage in the type definition. Unless specified otherwise, the default generic type is still the express object. It's inheriting from the NodeJS object, so it works fine. As a result, unless explicitly specified, there won't be any compatibility issues.

UPDATE:

Adding TypeScript tests turned out a little more complicated than I initially expected, hence only the one for mockRequest. In the process of writing them I ran into couple of issues:

eugef commented 10 months ago

Hi @mhaligowski thanks for the fix.

I wonder, how it will work in case Express-specific methods or properties are used.

For example:

const res = createResponse();
res.locals = {foo: 'bar'}

Since .locals is not part of OutgoingMessage type, won't it become an Typescript error?

mhaligowski commented 10 months ago

Hi @mhaligowski thanks for the fix.

I wonder, how it will work in case Express-specific methods or properties are used.

For example:

const res = createResponse();
res.locals = {foo: 'bar'}

Since .locals is not part of OutgoingMessage type, won't it become an Typescript error?

In the simple form, i.e. const res = createResponse(), the default return type is still Mock<Response>. However, with my change it's possible to call for example const res = createResponse<NextApiResponse>(), and the mock type will be Mock<NextApiResponse>. In the current implementation the NextApiResponse is not accepted by TypeScript because it doesn't extend express.Response class.

I think that the best way will be to just a TypeScript test to the test suite, but I don't really want to mess it up too bad with this PR. Let me see if I can modify the test routine so that we can start adding TypeScript tests as well.

mhaligowski commented 10 months ago

@eugef I added a PR with capability of writing TS test! Let me know if it's fine, and I'll rebase this PR on top of the TypeScript tests and will add some tests to make sure the current change works fine!

eugef commented 10 months ago

Hi @mhaligowski PR #282 looks good to me.

Feel free to proceed and add some tests.

mhaligowski commented 10 months ago

Phew! Those tests were bigger than expected! Let me update the PR body with the explanation what I did.

mhaligowski commented 10 months ago

Hey @mhaligowski, big thanks for the tremendous amount of work you've put in!

This PR looks solid from my end. If you believe it's ready to be merged, I'll go ahead, and release a new minor version.

I think so, although it's still my first time messing with .d.ts files. Would it make sense to have a pre-release? I don't want to risk breaking the package. The code is tested, but not sure if there's test coverage for package.

eugef commented 10 months ago

I'd suggest to make a pre-relase locally just for yourself and try this new version in some of your TS projects. I think this would be sufficient check

mhaligowski commented 10 months ago

I'd suggest to make a pre-relase locally just for yourself and try this new version in some of your TS projects. I think this would be sufficient check

Just did that!

I ran npm pack with a changed version, then imported into my project with "file://path/to/file.tgz. The problem magically disappeared :)

Good to go from me!

eugef commented 10 months ago

Great, thanks for testing.

Could you also add a section to readme that explains how to use this lib with NextApiRequest and TS

mhaligowski commented 9 months ago

Could you also add a section to readme that explains how to use this lib with NextApiRequest and TS

Opened #284!

Meags27 commented 9 months ago

Is it possible to add documentation on how to use NextResponse with next.js's App router? as I keep getting TypeError: request.json is not a function as an error

const mockRequestData = { emailToUpdate: "test@example.com" };
    const { req, res } = createMocks({
      method: "POST",
      url: "/api/account/email/save",
      body: mockRequestData,
    });
  try {
    const { emailToUpdate } = await request.json();

    return NextResponse.json({ reauthenticated: true }, { status: 200 });
  } catch (error) {
    console.error("Error updating Cognito email", error);
  }
}