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

Support Remix / React Router v7 #20790

Open
birkskyum opened this issue Oct 4, 2023 · 16 comments
Open

Support Remix / React Router v7 #20790

birkskyum opened this issue Oct 4, 2023 · 16 comments
Labels
bug Something isn't working correctly node compat

Comments

@birkskyum
Copy link
Contributor

birkskyum commented Oct 4, 2023

Remix - Website - GitHub

Platform

Darwin 23.0.0 arm64 arm

Version

deno 1.37.1

Repro

Anywhere, run deno run -A npm:@remix-run/dev build, or even just deno run -A npm:@remix-run/dev

Expected

In a proper app, like in the getting started guide here, the expected state would be similar to:

➜ npx remix build
 info  building... (NODE_ENV=production)
 info  built (113ms)

Actual

➜ deno run -A npm:@remix-run/dev      
error: Uncaught Error: Not implemented: Script.prototype.runInNewContext
    at notImplemented (ext:deno_node/_utils.ts:9:9)
    at Script.runInNewContext (node:vm:22:5)
    at Object.<anonymous> (file:///Users/admin/repos/deno-kitchensink/node_modules/.deno/eval@0.1.8/node_modules/eval/eval.js:17:4)
    at Object.<anonymous> (file:///Users/admin/repos/deno-kitchensink/node_modules/.deno/eval@0.1.8/node_modules/eval/eval.js:92:4)
    at Module._compile (node:module:733:34)
    at Object.Module._extensions..js (node:module:747:10)
    at Module.load (node:module:658:32)
    at Function.Module._load (node:module:539:12)
    at Module.require (node:module:677:19)
    at require (node:module:791:16)
@birkskyum birkskyum changed the title Support Remix.run Support Remix Oct 4, 2023
@bartlomieju bartlomieju added bug Something isn't working correctly node compat labels Oct 9, 2023
@bartlomieju
Copy link
Member

#21527 will be a step towards supporting this, but most likely not enough (as the PR doesn't support any options).

@birkskyum
Copy link
Contributor Author

birkskyum commented Dec 19, 2023

Things have changed with 1.39.

Using the standard (node) template (deno run -A npm:create-remix), and running deno task dev now yields:

➜ deno task dev
Task dev remix dev --manual

 💿  remix dev

 info  building...
 info  built (374ms)
The following error is a bug in Remix; please open an issue! https://github.com/remix-run/remix/issues/new
error: Uncaught Error: errno missing
    at Object.invariant [as default] (file:///Users/admin/repos/deno-kitchensink/my-remix-app/node_modules/.deno/@remix-run+dev@2.4.0/node_modules/@remix-run/dev/dist/invariant.js:18:11)
    at ChildProcess.<anonymous> (file:///Users/admin/repos/deno-kitchensink/my-remix-app/node_modules/.deno/@remix-run+dev@2.4.0/node_modules/@remix-run/dev/dist/devServer_unstable/index.js:150:27)
    at ChildProcess.emit (ext:deno_node/_events.mjs:395:35)
    at ext:deno_node/internal/child_process.ts:238:12
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:25:11)
    at runNextTicks (ext:deno_node/_next_tick.ts:71:3)
    at eventLoopTick (ext:core/01_core.js:188:21)

update: this deno template has been removed from github
Using the deno template instead: deno run -A npm:create-remix --template remix-run/remix/templates/deno, and running deno task dev:

➜ deno task dev  
Task dev npm-run-all build --parallel "dev:*"
Import map diagnostics:
  - Invalid top-level key "comment". Only "imports" and "scopes" can be present.
  - Invalid address "" for the specifier key "// `@remix-run/deno` code is already a Deno module, so just get types for it directly from `node_modules/`".

> build
> remix build

sh: remix: command not found
ERROR: "build" exited with 127.

@birkskyum
Copy link
Contributor Author

birkskyum commented Jan 28, 2024

UPDATE: This is resolved (duplicate of #21820), new errors shown in newer comments below

I tried the new vite mode of remix.

deno run -A npm:create-remix@latest --template remix-run/remix/templates/unstable-vite

.. and got this:

➜ deno task dev
Task dev remix vite:dev
failed to load config from /Users/admin/repos/deno-kitchensink/my-remix-vite/vite.config.ts
SyntaxError: The requested module 'file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/@remix-run/dev/dist/index.js' does not provide an export named 'unstable_vitePlugin' at file:///Users/admin/repos/deno-kitchensink/my-remix-vite/vite.config.ts.timestamp-1706464730208-0ec52d94af83c.mjs:2:10
    at async loadConfigFromBundledFile (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:68383:21)
    at async loadConfigFromFile (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:68240:28)
    at async resolveConfig (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:67841:28)
    at async _createServer (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:60327:20)
    at async dev (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/@remix-run/dev/dist/vite/dev.js:39:16)
    at async Object.viteDev (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/@remix-run/dev/dist/cli/commands.js:213:3)
    at async Object.run (file:///Users/admin/repos/deno-kitchensink/my-remix-vite/node_modules/.deno/@remix-run+dev@2.5.1/node_modules/@remix-run/dev/dist/cli/run.js:224:7) {
  code: "ERR_MODULE_NOT_FOUND"
}

related:

@arjunyel
Copy link

@birkskyum hi friend just curious if you got the Vite version of Remix working with Deno?

@birkskyum
Copy link
Contributor Author

birkskyum commented Feb 11, 2024

@arjunyel , no I tried few days ago again but didn't manage unfortunately, but I'm glad to see this ticket mentioned in here:

@birkskyum
Copy link
Contributor Author

@birkskyum
Copy link
Contributor Author

birkskyum commented May 11, 2024

I'm now (deno 1.43.3) getting this for the standard remix (vite-based) template:

$ deno run -A npm:create-remix@latest --template remix-run/remix/templates/remix
$ deno task dev

Task dev remix vite:dev
  ➜  Local:   http://localhost:5173/
  ➜  Network: use --host to expose
  ➜  press h + enter to show help
TypeError: Return value from serve handler must be a response or a promise resolving to a response
    at ext:deno_http/00_serve.ts:334:15
    at Object.runMicrotasks (ext:core/01_core.js:642:26)
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:39:10)
    at eventLoopTick (ext:core/01_core.js:165:29)
Exception in onError while handling exception TypeError: Return value from onError handler must be a response or a promise resolving to a response
    at ext:deno_http/00_serve.ts:340:17
    at Object.runMicrotasks (ext:core/01_core.js:642:26)
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:39:10)
    at eventLoopTick (ext:core/01_core.js:165:29)
Terminating Deno.serve loop due to unexpected error TypeError: Cannot read properties of undefined (reading 'status')
    at ext:deno_http/00_serve.ts:361:26
    at Object.runMicrotasks (ext:core/01_core.js:642:26)
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:39:10)
    at eventLoopTick (ext:core/01_core.js:165:29)

@birkskyum birkskyum changed the title Support Remix Support Remix / React Route v7 May 18, 2024
@birkskyum birkskyum changed the title Support Remix / React Route v7 Support Remix / React Router v7 May 18, 2024
@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 10, 2024

With DENO_FUTURE=1 the dev task appear to work now! Serving a build still breaks though, as described below

➜ deno run -A npm:create-remix@latest --template remix-run/remix/templates/remix

select ts, install with npm etc.

➜ DENO_FUTURE=1 deno task build
➜ DENO_FUTURE=1 deno task start

Got this error:

Task start remix-serve ./build/server/index.js
error: Uncaught (in promise) SyntaxError: The requested module 'react-dom/server' does not provide an export named 'renderToPipeableStream' at file:///Users/admin/repos/deno-kitchensink/my-remix-app2/build/server/index.js?t=1723290078885:6:10
    at async run (file:///Users/admin/repos/deno-kitchensink/my-remix-app2/node_modules/@remix-run/serve/dist/cli.js:111:15)

@marvinhagemeister
Copy link
Contributor

This seems to be a bug in the react-dom package. They have a deno import condition that points to the browser bundle instead of the node bundle in their package.json. When I point it to the node bundle it works.

https://github.com/facebook/react/blob/8d74e8c73a5cc5e461bb1413a74c6b058c6be134/packages/react-dom/package.json#L64

    "./server": {
      "react-server": "./server.react-server.js",
      "workerd": "./server.edge.js",
      "bun": "./server.bun.js",
      "deno": "./server.browser.js",
      "worker": "./server.browser.js",
      "node": "./server.node.js",
      "edge-light": "./server.edge.js",
      "browser": "./server.browser.js",
      "default": "./server.node.js"
    },

@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 12, 2024

@marvinhagemeister , the ticket was closed. As an outsider it looks a bit like there's a discrepancy between how the react / deno team thinks deno should behave. I can just ascertain that as a user this breaks by default - or maybe I just misunderstood how the node compat in deno works, that's also a very real possibility :)

@marvinhagemeister
Copy link
Contributor

Yeah, the thing is that Deno supports both: Node streams and web streams. The way the react package is constructed is that they expect an "either or", not both. The renderToPipeableStream function is only supported in their node adapter. It seems like the proper solution would be to add a dedicated Deno entry instead of linking it to the browser one. Not sure if the react renderer configuration supports using both at the same time.

@redabacha
Copy link

the current deno template on remix is very outdated, i've opened a pr here to modernize it. it's based on this template i made https://github.com/redabacha/remix-deno-vite-template which has deno + remix working in a deno-first way.

i don't think there's anything that can be fixed on the deno side of things here as the bug is caused by react-dom making poor assumptions about what api's should be available and the remix + deno story not being given any love in a few years. remix can provide a first class solution for this and i'd rather it go that route than forcing deno to use node.js compat for a framework that's supposed to be all about using web standards.

@OnielN14
Copy link

Got this error when using renderToPipeableStream from react-dom/server.node instead of react-dom/server :

[remix-serve] http://localhost:3000 (http://172.16.0.2:3000)
TypeError: Return value from serve handler must be a response or a promise resolving to a response
    at ext:deno_http/00_serve.ts:372:15
    at eventLoopTick (ext:core/01_core.js:214:9)
Exception in onError while handling exception TypeError: Return value from onError handler must be a response or a promise resolving to a response
    at ext:deno_http/00_serve.ts:384:17
    at eventLoopTick (ext:core/01_core.js:214:9)
Terminating Deno.serve loop due to unexpected error TypeError: Cannot read properties of undefined (reading 'status')
    at ext:deno_http/00_serve.ts:410:26
    at eventLoopTick (ext:core/01_core.js:214:9)
GET / 200 - - 30.205 ms

The error occurs after I try to access the web, then the server forced to close.

here is modified entry.server.tsx :

/**
 * By default, Remix will handle generating the HTTP Response for you.
 * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨
 * For more information, see https://remix.run/file-conventions/entry.server
 */

import { PassThrough } from "node:stream";

import type { AppLoadContext, EntryContext } from "@remix-run/node";
import { createReadableStreamFromReadable } from "@remix-run/node";
import { RemixServer } from "@remix-run/react";
import { isbot } from "isbot";
- import { renderToPipeableStream } from "react-dom/server";
+ import { renderToPipeableStream } from "react-dom/server.node";

const ABORT_DELAY = 5_000;

export default function handleRequest(
  request: Request,
  responseStatusCode: number,
  responseHeaders: Headers,
  remixContext: EntryContext,
  // This is ignored so we can keep it in the template for visibility.  Feel
  // free to delete this parameter in your app if you're not using it!
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  loadContext: AppLoadContext
) {
  return isbot(request.headers.get("user-agent") || "")
    ? handleBotRequest(
        request,
        responseStatusCode,
        responseHeaders,
        remixContext
      )
    : handleBrowserRequest(
        request,
        responseStatusCode,
        responseHeaders,
        remixContext
      );
}

function handleBotRequest(
  request: Request,
  responseStatusCode: number,
  responseHeaders: Headers,
  remixContext: EntryContext
) {
  return new Promise((resolve, reject) => {
    let shellRendered = false;
    const { pipe, abort } = renderToPipeableStream(
      <RemixServer
        context={remixContext}
        url={request.url}
        abortDelay={ABORT_DELAY}
      />,
      {
        onAllReady() {
          shellRendered = true;
          const body = new PassThrough();
          const stream = createReadableStreamFromReadable(body);

          responseHeaders.set("Content-Type", "text/html");

          resolve(
            new Response(stream, {
              headers: responseHeaders,
              status: responseStatusCode,
            })
          );

          pipe(body);
        },
        onShellError(error: unknown) {
          reject(error);
        },
        onError(error: unknown) {
          responseStatusCode = 500;
          // Log streaming rendering errors from inside the shell.  Don't log
          // errors encountered during initial shell rendering since they'll
          // reject and get logged in handleDocumentRequest.
          if (shellRendered) {
            console.error(error);
          }
        },
      }
    );

    setTimeout(abort, ABORT_DELAY);
  });
}

function handleBrowserRequest(
  request: Request,
  responseStatusCode: number,
  responseHeaders: Headers,
  remixContext: EntryContext
) {
  return new Promise((resolve, reject) => {
    let shellRendered = false;
    const { pipe, abort } = renderToPipeableStream(
      <RemixServer
        context={remixContext}
        url={request.url}
        abortDelay={ABORT_DELAY}
      />,
      {
        onShellReady() {
          shellRendered = true;
          const body = new PassThrough();
          const stream = createReadableStreamFromReadable(body);

          responseHeaders.set("Content-Type", "text/html");

          resolve(
            new Response(stream, {
              headers: responseHeaders,
              status: responseStatusCode,
            })
          );

          pipe(body);
        },
        onShellError(error: unknown) {
          reject(error);
        },
        onError(error: unknown) {
          responseStatusCode = 500;
          // Log streaming rendering errors from inside the shell.  Don't log
          // errors encountered during initial shell rendering since they'll
          // reject and get logged in handleDocumentRequest.
          if (shellRendered) {
            console.error(error);
          }
        },
      }
    );

    setTimeout(abort, ABORT_DELAY);
  });
}

@redabacha
Copy link

redabacha commented Oct 12, 2024

@OnielN14 this occurs due to remix-serve cli blindly installing its own global polyfills (via @remix/node/install) instead of actually checking if it is necessary to do so or not. i have fixed this issue in the aforementioned pr here.

as for a workaround, i'd suggest either using the official express template which works without the remix-serve cli or use my template here :)

@OnielN14
Copy link

Hi @redabacha,
I have follow your changes on entry.server.tsx and add server.production.ts only, now it's work properly, I can access the web without forcing the server to close.

Thanks for your hard work!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

No branches or pull requests

6 participants