fix(remix-dev/vite): use global Request class in Vite dev server handler

[hi-ogawa]

Closes: #7819, #8050

Currently in vite dev, remix handler makes use of NodeRequest class (based on undici) which is a different from global Request and this breaks user code relying on ... instanceof Request.

I noticed that this issue doesn't exist on express custom server since it uses its own way to construct Request instance based on global Request in packages/remix-express/server.ts. So, I was wondering if it would make sense to do something similar for vite in packages/remix-dev/vite/node/adapter.ts as well.

Probably there is a reason to use handler code based on solid-start, so if there is a known issue with this approach, please let me know.

testing strategy

I added a new test integration/vite-dev-custom-entry-test.ts to verify request instance Request is true during server entry's handleRequest.

[changeset-bot]

🦋 Changeset detected

Latest commit: f4b3f34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@remix-run/dev	Patch
create-remix	Patch
remix	Patch
@remix-run/architect	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/css-bundle	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/express	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/serve	Patch
@remix-run/server-runtime	Patch
@remix-run/testing	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[markdalgleish]

Thanks so much for the PR! I love that this gets us much closer to the other adapters.

[hi-ogawa]

Thanks for proceeding with this PR!

I was wondering about what's the future plan of this forced installGlobals during vite dev.
Node 18 ships fetch, Request, etc... globals, so I thought forcing polyfills here might not be appropriate and better to be opt-in choice by individual users.

Looking at what happened in other adapters #7009 (and also unstable-vite-express currently has explicit installGlobals in server.mjs), the general direction seems to remove automatic/forced installGlobals.

Currently this polyfills kicks in whenever remix vite plugin is loaded, so I think it's not even possible for custom server (during dev) to opt-out from installGlobals currently.

Here is a quick repro:

https://stackblitz.com/edit/remix-run-remix-ws1uig?file=vite.config.ts

//
// vite.config.ts
//

let fetch1 = fetch;

import { unstable_vitePlugin as remix } from '@remix-run/dev';
import { defineConfig } from 'vite';
import tsconfigPaths from 'vite-tsconfig-paths';

let fetch2 = fetch;

export default defineConfig({
  clearScreen: false,
  plugins: [remix(), tsconfigPaths()],
});

let fetch3 = fetch;

// the log happens twice in parent and child compilers
//   parent: true false
//   child:  true true
console.log(fetch1 === fetch2, fetch1 === fetch3);

EDIT: Current behavior would be essentially same as installGlobals in vite.config.ts, so in order to allow users to opt-out, I think I would prefer doing it in the template's vite.config.ts #8119

[markdalgleish]

Yep, I was wanting to follow this up separately. Thanks for looking into this!

[github-actions]

🤖 Hello there,

We just published version 2.4.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.4.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!