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

New Defer API does not work with blues-stack #151

Closed
justinwaite opened this issue Jan 21, 2023 · 11 comments
Closed

New Defer API does not work with blues-stack #151

justinwaite opened this issue Jan 21, 2023 · 11 comments

Comments

@justinwaite
Copy link
Contributor

justinwaite commented Jan 21, 2023

Have you experienced this bug with the latest version of the template?

Yes

Steps to Reproduce

Setup the project
Create account and sign in
Update loader in notes.tsx to return defer instead of json

Expected Behavior

I would expect that just changing to defer while still resolving the promise in the loader would still work without any other changes, since the data is being awaited in the loader.

Actual Behavior

The page errors with TypeError: Cannot read properties of undefined (reading 'length')

When looking at the value of data from const data = useLoaderData<typeof loader>();, it looks like this:

DeferredData {
  pendingKeysSet: Set(0) {},
  subscribers: Set(0) {},
  deferredKeys: [],
  abortPromise: Promise { <pending> },
  controller: AbortController { signal: AbortSignal { aborted: false } },
  unlistenAbortSignal: [Function (anonymous)],
  data: { noteListItems: [] },
  init: { headers: { 'content-type': 'application/json; charset=utf-8' } }
}

This shape seems wrong to me. You should have to access the real data via data.data, right?

@justinwaite
Copy link
Contributor Author

@machour
Copy link
Collaborator

machour commented Jan 21, 2023

switching from json to defer is not enough, you have to wrap the deferred data in <Await />

See our guide on streaming: https://remix.run/docs/en/v1/guides/streaming#the-solution

@machour machour closed this as not planned Won't fix, can't repro, duplicate, stale Jan 21, 2023
@justinwaite
Copy link
Contributor Author

@machour I would encourage you to reopen – If you look at the repo (and my instructions above), I'm awaiting the data in the loader, so it should be "instantly" available.

I did find the issue – it's the --bundle flag on the build:server script. If you remove it, everything works as expected.

@justinwaite
Copy link
Contributor Author

If you want, feel free to remove await in the loader and add Suspense and Await in the component. You just don't need to go that far to see the error. The result is the same.

@machour
Copy link
Collaborator

machour commented Jan 21, 2023

🤦🏼‍♂️ sorry @justinwaite, reopened.

@machour machour reopened this Jan 21, 2023
@JDouven
Copy link

JDouven commented Feb 16, 2023

Any new info regarding this? I'm running into this as well but removing --bundle is not an option for me.

@MichaelDeBoey
Copy link
Member

@justinwaite @JDouven Does it work with the default Remix template?
https://github.com/remix-run/remix/tree/main/templates/remix

@JDouven
Copy link

JDouven commented Feb 16, 2023

@MichaelDeBoey Yes, that works. In my case I serve the app with express instead of remix-serve.

I used the following code:

import { defer } from '@remix-run/node';
import { Await, useLoaderData } from '@remix-run/react';
import { Suspense } from 'react';

export async function loader() {
  const testPromise = new Promise<string>((resolve) => {
    setTimeout(() => {
      resolve('Hello with a delay!');
    }, 3000);
  });

  return defer({
    instantly: 'foo',
    testPromise,
  });
}

export default function Index() {
  const data = useLoaderData<typeof loader>();
  return (
    <div style={{ fontFamily: 'system-ui, sans-serif', lineHeight: '1.4' }}>
      <h1>Welcome to Remix</h1>
      <h2>Instant data:</h2>
      <pre>{data.instantly}</pre>
      <h2>Streamed data:</h2>
      <Suspense fallback={<p>Suspense fallback</p>}>
        <Await
          resolve={data.testPromise}
          errorElement={<p>Await errorElement</p>}
        >
          {(testPromise) => <pre>{testPromise}</pre>}
        </Await>
      </Suspense>
    </div>
  );
}

So I used the express template, and that works too. Then I changed the following to make it more closely match the relevant parts of blues-stack:

  • Rename server.js to server.ts and fix the imports (and npm install missing types)
  • Update package.json to:
"scripts": {
    "build": "remix build",
    "build:server": "esbuild --platform=node --format=cjs ./server.ts --outdir=build --bundle",
    "dev": "run-p dev:*",
    "dev:remix": "remix watch",
    "dev:build": "cross-env NODE_ENV=development npm run build:server -- --watch",
    "dev:server": "cross-env NODE_ENV=development node --inspect --require ./node_modules/dotenv/config ./build/server.js",
}

(inspired by the blues-stack package.json)

When I now run npm run dev my example stops working. The frontend stops receiving loader data completely. Neither the 'instant' data nor the streamed data is visible.
Also in this small example, removing --bundle from the build:server script is a workaround. However as I said, that is not an option in the larger app I want to use this in.

Let me know if you need more info.

@brophdawg11
Copy link
Contributor

This looks to be due to a failing instanceof check across a CJS/ESM boundary when using --bundle. We'll get a fix added for this but in the meantime if you'd like to get past it via patch-package you just need to replace the single check in @remix-run/router:

  // before
  if (result instanceof DeferredData) {

  // after
  if (result &&
      typeof result === "object" &&
      typeof result.data === "object" &&
      typeof result.subscribe === "function" &&
      typeof result.cancel === "function" &&
      typeof result.resolveData === "function") {

You should patch this in all 3 build files to be safe:

  • @remix-run/router/dist/router.js
  • @remix-run/router/dist/router.cjs.js
  • @remix-run/router/dist/router.umd.js

@JDouven
Copy link

JDouven commented Mar 23, 2023

@brophdawg11 I tried this and I can confirm this fixes the issue! Thanks!

@brophdawg11
Copy link
Contributor

The fix for this is out in 1.15.0-pre.1 if anyone wants to give that a spin. We should have a stable 1.15.0 out soon as well.

@mcansh mcansh closed this as completed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants