@remix-run/web-fetch causes tests to error after upgrade to jest 28

[penx]

What version of Remix are you using?

1.5.1

Steps to Reproduce

• update jest-related dependencies to v28 as per chore: update jest-related dependencies to v28 #3400
• add transformIgnorePatterns as per chore: update jest-related dependencies to v28 #3400 (comment)
• run yarn test:primary

Expected Behavior

Tests should pass

Actual Behavior

    TypeError: Class extends value undefined is not a constructor or null

      34 | class NodeRequest extends WebRequest {
      35 |   constructor(info: NodeRequestInfo, init?: NodeRequestInit) {
    > 36 |     super(info, init as RequestInit);
         |                                     ^
      37 |   }
      38 |

This seems to relate to the types output by @remix-run/web-fetch:

export { default, fetch, Headers, Request, Response } from "./fetch.js";
export { ReadableStream, Blob, FormData } from "./package.js";
//# sourceMappingURL=lib.node.d.ts.map

node_modules/@remix-run/web-fetch/dist/src/lib.node.d.ts

fetch.js and package.js do not exist in the distributed @remix-run/web-fetch and should either be included or lib.node.d.ts should be pointing to fetch.d.ts and package.d.ts

[MichaelDeBoey]

@penx You can always create a PR to https://github.com/remix-run/web-std-io if you want.

[penx]

On closer inspection, vscode's ts server seems to be able to resolve WebRequest to src/request.js (via dist/src/lib.node.d.ts, I assume via the source map), whereas Jest (or babel) is complaining, so perhaps this is a babel or config issue.

[MichaelDeBoey]

@penx Would be nice if we could find the root cause.

[penx]

I'm looking 😄 if I can't figure it out soon I may need to look again later in the week. My understanding is Jest is configured to use Babel to compile the TS, which is reporting the TypeError due to not being able to resolve the type via the sourcemap, wheras TypeScript manages to do this.

edit: having said that, I don't think Babel checks types...

I haven't yet found any issues on babel referencing this.

I am wondering whether ts-jest has been considered?

[penx]

I haven't completely traced the source of the issue but it seems changing

    ".": {
      "browser": "./src/lib.js",
      "require": "./dist/lib.node.cjs",
      "import": "./src/lib.node.js",
      "types": "./dist/src/lib.node.d.ts"
    },

to

    ".": {
      "require": "./dist/lib.node.cjs",
      "import": "./src/lib.node.js",
      "types": "./dist/src/lib.node.d.ts",
      "browser": "./src/lib.js"
    },

...in the @remix-run/web-fetch package.json seems to resolve this issue.

There are continued Jest issues after this though so I'll try to keep digging.

[penx]

Ok I have test:primary all passing via either this change above or adding to transformIgnorePatterns

  transformIgnorePatterns: [
    "/node_modules/(?!(@remix-run/web-fetch|@remix-run/web-blob|@remix-run/web-stream|@remix-run/web-form-data|@remix-run/web-file|@web3-storage/multipart-parser)/)",
  ],

I was going to suggest the above change anyway as it the exports order is important and I don't understand why the browser export would appear first (or why it would point to the source) so can raise a PR for that.

[penx]

remix-run/web-std-io#11

[MichaelDeBoey]

Read something about Community Conditions Definitions today (can't remember where though 😅), which says types should always be first.
So that might already be a good thing to change.

https://nodejs.org/api/packages.html#community-conditions-definitions

[penx]

Changing the order to

      "types": "./dist/src/lib.node.d.ts",
      "browser": "./src/lib.js",
      "require": "./dist/lib.node.cjs",
      "import": "./src/lib.node.js"

Still results in the same issue, though

      "types": "./dist/src/lib.node.d.ts",
      "require": "./dist/lib.node.cjs",
      "import": "./src/lib.node.js",
      "browser": "./src/lib.js"

It is fixed as before. I'd read about the types export recently too - I'll update my PR to do that.

[MichaelDeBoey]

@penx Yeah I think you referenced it somewhere.
Just can't find it anymore 😕

[penx]

"types" - can be used by typing systems to resolve the typing file for the given export. This condition should always be included first.

https://nodejs.org/api/packages.html

Not sure if this is what you mean you couldn't find as that's what you just linked to :-)

[MichaelDeBoey]

@penx I meant the thing you referenced that was referencing Community Conditions Definitions.

[MichaelDeBoey]

@penx It was mentioned in GoogleCloudPlatform/functions-framework-nodejs#461 (comment).

Do we still need the update jest to v28 after that change?

[penx]

Ah ok, I was about to link to these which I've been looking at over last couple days

• https://nodejs.org/api/esm.html#esm_main_entry_point_export
• Support exports syntax in package.json evanw/esbuild#187

[penx]

Do we still need the update jest to v28 after that change?

yes I am pretty sure the jest update is still needed so that it can resolve

import { getTestServer } from "@google-cloud/functions-framework/testing";

From what I remember, before Jest 28, Jest won't look for exports defined in package.json

[penx]

Although I'm not sure why GoogleCloudPlatform/functions-framework-nodejs#461 is needed - when the d.ts files are in the same folder as the source they should be resolved automatically.

[spencersmb]

I was able to get most tests working following this thread, but I still cannot get a simple fetch request to pass. I always get this error:

TypeError: webBlob.ReadableStream is not a constructor

      at fromStream (node_modules/@remix-run/web-fetch/src/body.js:482:17)
      at new Body (node_modules/@remix-run/web-fetch/src/body.js:96:17)
      at new Response (node_modules/@remix-run/web-fetch/src/response.js:30:3)
      at ClientRequest.<anonymous> (node_modules/@remix-run/web-fetch/src/fetch.js:262:16)

  ● MSW test with Remix 1.5.1

    Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close

I'm transforming these files within jest:

transformIgnorePatterns: [
    "/node_modules/(?!(@remix-run/web-fetch|@remix-run/web-blob|@remix-run/web-stream|@remix-run/web-form-data|@remix-run/web-file|@web3-storage/multipart-parser)/)"
  ],
  transform: {
    "^.+\\.(js|ts)$": "ts-jest",
  },

Here is my fetch request:

export const fetchExampleTest = async () => {
  const url = 'https://jsonplaceholder.typicode.com/todos/1';
  return await fetch(url, {
    method: 'GET',
    headers: {
      'Content-Type': 'application/json',
    }
  })
}

Once I import this into the test and try to run it - I get that error. Here is my repo:
https://github.com/spencersmb/remix-msw

[machour]

@penx is this still a problem for you?

[penx]

This was preventing the upgrade from Jest 27 to 28+

Remix is still using Jest 27 and the upstream PR I raised to resolve this has not been merged, so I am pretty sure it is still an issue.

remix-run/web-std-io#11

[vpfaulkner]

For what it's worth, we're running into this exact same issue and had to downgrade to Jest 27. I guess we're blocked until Remix upgrades to Jest 28?

    /Users/vpfaulkner/Documents/Code/yellowhat/appy/node_modules/@remix-run/web-fetch/src/lib.js:4
    export { ReadableStream, Blob, FormData  } from './package.js';
    ^^^^^^

    SyntaxError: Unexpected token 'export'

      1 | import type { ActionArgs } from "@remix-run/node"
    > 2 | import { json } from "@remix-run/node"

Our Jest config:

module.exports = {
  preset: "ts-jest",
  testEnvironment: "jsdom",
  transform: {
    "^.+\\.(ts|tsx)?$": "ts-jest",
  },
  moduleNameMapper: {
    "^~/(.*)$": "<rootDir>/app/$1",
  },
}

[zmdavis]

We ran into the same problem when attempting to upgrade to Jest 28 (or Jest 29 for that matter). I have not tested the fix in remix-run/web-std-io#11 but that seems like a simple fix to unblock those of us who would like to upgrade Jest.

[MichaelDeBoey]

The actual fix wouldn't be remix-run/web-std-io#11, but remix-run/web-std-io#43 instead