smeijer / next-runtime Goto Github PK

Thank you very much for the great library!
This library has accelerated my team's efforts to improve the user experience by moving business logic code from the client side to the server side. At the same time, I am also developing with Remix, and I feel that I am incorporating the best parts of Remix into Next.js.

To get to the point, when using Form components and the submit endpoint (post handler) is returning a redirect, the fetch and navigation functions make two requests to the redirect destination. I understand that the fetch function automatically makes the redirect request because it is a forced specification of the browser, and that I have no control over this. So we see this as unavoidable, but the final latency is doubled, which can be fatal if there is a time-consuming data fetch in the get handler.
Currently, we work around this problem by implementing a handler that references the accept header and immediately returns a response if it is "json".

Is there any other smart solution to this problem? Or, can you implement a mode to determine that the redirect is due to a form submit and return a response immediately?

Thank you in advance.

Hi! First of all, this is a great little library, so thank you for making and maintaining this.

One issue I ran into when using the error value returned from useFormSubmit is that it's type is FetchError | Error | undefined, but in Typescript, there is no way to check which one it is without importing FetchError, which is not currently a part of the public API:

import { FetchError } from "next-runtime/lib/fetch-error";

I wrote a little type guard for it like so:

function isFetchError(err: unknown): err is FetchError {
  return err instanceof FetchError;
}

Unfortunately, this could break in the future since FetchError is not exported from index.js, so I was wondering if we could add that?

The hook should return a submit method, so we can submit data without using a form element.

This can be useful for automated things, like calling an api after a certain timeout (markAsRead) or for event tracking (userClickedLink).

Critical that this onSubmit accepts an action parameter.

I was thinking to add basic session support, something self baked or using cookie-session. Haven't thought it through yet.

That would mean that the context would get an session prop.

export const getServerSideProps = handle({
  async get({ session }) {
    session.get('user');
    session.set('user', 'smeijer');
  },
});

I have created a small example.
https://codesandbox.io/s/goofy-allen-jixz0k?file=/pages/index.js

/destination1 (getServerSideProps is defined which returns empty json): redirect works fine.

import { handle, json } from "next-runtime";

export const getServerSideProps = handle({
  get: async () => {
    return json({});
  }
});

export default function ClientRedirect() {
  return <p>The redirect has landed. 🚀</p>;
}

/destination2 (getServerSideProps is not defined): redirect does not work.

export default function ClientRedirect() {
  return <p>The redirect has landed. 🚀</p>;
}

Both of these work when using <form> request.

Here's what I get:

Server Error
Error: Additional keys were returned from `getServerSideProps`. Properties intended for your component must be nested under the `props` key, e.g.:

	return { props: { title: 'My Title', content: '...' } }

Keys that need to be moved: size, timeout, __next_type__.
Read more: https://nextjs.org/docs/messages/invalid-getstaticprops-value
This error happened while generating the page. Any console logs will be displayed in the terminal window.
Call Stack
Object.renderToHTML
file:///C:/dev/thoughtbucket-next/node_modules/.pnpm/[email protected]_bd7797d63db473de6e2318609a63c933/node_modules/next/dist/server/render.js (464:23)
async doRender
file:///C:/dev/thoughtbucket-next/node_modules/.pnpm/[email protected]_bd7797d63db473de6e2318609a63c933/node_modules/next/dist/server/next-server.js (1144:38)
async
file:///C:/dev/thoughtbucket-next/node_modules/.pnpm/[email protected]_bd7797d63db473de6e2318609a63c933/node_modules/next/dist/server/next-server.js (1236:28)
async
file:///C:/dev/thoughtbucket-next/node_modules/.pnpm/[email protected]_bd7797d63db473de6e2318609a63c933/node_modules/next/dist/server/response-cache.js (64:36)

If extra metadata is needed on the response, would a symbol key work? Unless it needs to be serializable, then I'm not sure 🤔

I got a bit confused from the docs. The main example doesn't show the message after making the post. You can fix this by using lower case form, reading the data from the hook, or adding withNextRuntime. The note on the last docs page suggests the second option. Should the other pages reflect this?

Right now, I have to do this:

  const pending = usePendingFormSubmit()
  const newColumnName = pending?.data.get("createColumn.name")
  const deletedColumnId = pending?.data.get("deleteColumn.id")

Which makes sense, as those are the actual names of the fields that I'm using, but it'd be convenient to have the body on the client in the same object shape as the body on the server, with the nested fields. So, I could do pending?.body.createColumn?.name, and reuse the same body type on the client. We should also keep the actual form data around too, just in case it's needed for whatever reason ¯\_(ツ)_/¯

I went to see what would happen when I did, and a lot of errors showed up 😅 This can be addressed as its own task, and is definitely worth doing for safer internal code

First, thank you for such a nice library 🎉

There is a type error when I pass ref to the <Form> component (but, it works well, just a type error).

repro: https://stackblitz.com/edit/nextjs-9mtrvu?file=pages/index.tsx

The signature right now:

function usePendingFormSubmit(): {
    state: "pending";
    data: FormData;
}

Needs to be nullable to enforce existence checks

This is a big problem when multiple pages have different forms, or when using withNextRuntime. Submit a form on route A, and the form data will replace the props on a different route B if you navigate to that page.

Are the next api routes not enough?

Upgraded to 2.0.0 this morning and started getting the below:

error - TypeError: Cannot read property 'headers' of undefined
    at new Accepts (/node_modules/accepts/index.js:37:22)
    at Accepts (/node_modules/accepts/index.js:34:12)
    at applyResponse (/node_modules/next-runtime/handle.js:40:35)
    at /node_modules/next-runtime/handle.js:111:36
    at Generator.next (<anonymous>)
    at fulfilled (/node_modules/next-runtime/handle.js:5:58)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:93:5) {
  page: '/dashboard'

Here is the handle implementation on my /dashboard page:

export const getServerSideProps = handle({
  get: async (context) => {
   // supabase query. truncated for brevity.
    const { error, data: organizations } = await supabaseClient....

    return json({ error, organizations, user: context.req.user });
  }
});

the above works as expected with the latest 1.x code when performing an HTML request, but errors on 2.0.0.

The error seems to terminate at the req.headers access in Accepts/index.js:

function Accepts (req) {
  if (!(this instanceof Accepts)) {
    return new Accepts(req)
  }

  this.headers = req.headers <<<<<<<< here
  this.negotiator = new Negotiator(req)
}

For things like billing flows, its not uncommon to want to redirect-after-POST to a 3rd party endpoint. next-runtime doesn't appear to support returning external URLs with its redirect handler.

I've even tried things like the following:

res.setHeader("location",  someExternalURL);
res.statusCode = 307;
res.end();
return;

But I get this error:

error - Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

Right now, our workaround is to return json and update window.location.href during render. It'd be a little neater if this scenario could be detected and managed automatically.

I might be misunderstanding or doing something wrong here 😅 but either way, here's my scenario:

• User logs in from the login page
• On success, they get redirected to another page
• That other page has a POST handler, which accepts data

Because the request to /login was a POST, the redirect is also a POST, and that POST redirect is sending the login data, which the other page's POST doesn't expect.

And there's nothing fancy going on in the login page, it's just a regular redirect:

export const getServerSideProps = handle<Props>({
  async post(context) {
    // check credentials + create session

    return redirect("/")
  },
})

I think what's needed here is being able to define the redirect method, e.g. redirect('/', { method: 'GET' }). Or maybe the redirect should always be a no-data GET? Or maybe the real solution is to move the other page's POST handler into a dedicated API route... 🤔 thoughts appreciated, I can also clarify anything here if need be

Ah, I should also mention that this is with JS disabled

At the moment, it appears as though the new page's props always come from GET. If so, this doesn't allow returning and using an error message from a json() in POST or PATCH. Next.js's data fetching mechanism is kind of blurry to me, so I'm not sure how this should be accomplished 🤔 I might also not be observing this correctly, but I can try to reproduce it later

For a nice pagination experience, Form should support various redirect methods. Currently, when using a Form with method="get" it will always redirect using replaceState:

In addition, we want to be able to:

• use pushState instead,
• not navigate at all

Solution in mind:

We currently default to replaceState, I think we should change that default to pushState instead. As that's what the browser does.

Then we can add a replace argument to <Form to switch it to replaceState. And we can support navigate={false} to prevent navigation.

Use pushState

<Form method="get">

Use replaceState:

<Form method="get" replace>

No navigation:

<Form method="get" navigate={false}>

Hi,
I love this library, very nifty :)

This issue may be too early for you to consider, but next-runtime does not work with a NextJS page using the experimental-edge runtime. https://nextjs.org/docs/advanced-features/react-18/switchable-runtime#page-runtime-option

Specific error message when building:

Failed to compile.
--
18:18:36.899 |  
18:18:36.899 | ./node_modules/body-parser/node_modules/destroy/index.js:16:0
18:18:36.899 | Module not found: Can't resolve 'fs'
18:18:36.899 |  
18:18:36.899 | Import trace for requested module:
18:18:36.899 | ./node_modules/body-parser/lib/read.js
18:18:36.899 | ./node_modules/body-parser/lib/types/json.js
18:18:36.900 | ./node_modules/body-parser/index.js
18:18:36.900 | ./node_modules/next-runtime/runtime/body-parser.js
18:18:36.900 | ./node_modules/next-runtime/handle.js
18:18:36.900 | ./node_modules/next-runtime/index.js
18:18:36.900 | ./src/pages/admin/payments/attempt/[idempotencyKey].tsx

It looks like body-parser has a hard dependency on fs for some reason, and file-system activity is not available in Edge functions.
You only use body-parser in one place, here: https://github.com/smeijer/next-runtime/blob/main/src/runtime/body-parser.ts#L104

Possible fixes:

• Maybe updating body-parser from 1.19.0 to 1.20.0 (I don't think this will work)
• Importing only json from 'body-parser (might not actually shake out the fs usage)
• Find an alternative existing library (I couldn't find an obvious one, and I don't know enough about how you're using it to suggest one)
• Write your own body parser (possibly very difficult?)
• Add to documentation that you won't / can't support the edge runtime.

Thanks for all your work here

This is a very unscientific demo, but it should help illustrate the point regardless:

This is my work in progress trello-like app. It uses the pending form submission to show placeholders while the actual update is happening in the backend.

At the moment, the form is considered submitted after the form request is completed. You can see this in the video: the pending states stop before the data is actually updated, and I've also included the devtool network requests for clarity.

I think it would be more helpful to wait until the form request and the navigation is complete for the form to be considered done

The Form#onError handler currently returns a string that's composed like [${response.statusCode]: ${response.statusMessage}, I think that's too opinionated and should be changed. The onError handler currently does also not include the response data.

I see two variants:

• onError(error: { code: string | number, message: string }, data: JSON })

  error, if the error is thrown at the server-side, this will be { code: response.status, message: response.statusText }. If it's a client-side error, it will be { code: error.name, message: error.message }.

  data will be the response object from the server, send by json(...)

• onError(error: { code: string | number, message: string, [key: string]: unknown })
  The idea is to merge the code and message from the other signature, with the response data. This way there is only one error object. On error status codes, we can assume that the data that's being sent along with the response, is part of the error. Right? It also allows the user to provide the error code and message from the server.

  As a bonus, it leaves space to use data for the submission data. That way we could maybe also include the submitted data in the error.

I actually didn't realize these were gone until I tried to use them 😆

So, from reading the docs, I figure the rationale was that the onSuccess/onError callbacks were used to update state to show some message in the UI. The useFormSubmit state works fine in that case. But when I need to fire some side effect, the hook state feels more cumbersome.

Here's an example and comparison. The modal accepts a render prop for a close function.

Without the onSuccess callback, I need to use a useEffect that relies on the hook state, which also requires making a new component in which to use the hook:

export function EditBucketButton({ bucket }) {
  return (
    <Modal
      title="edit bucket"
      renderContent={({ close }) => (
        <EditBucketForm bucket={bucket} onSuccess={close} />
      )}
    />
  )
}

function EditBucketForm({ bucket, onSuccess }) {
  const { isSuccess } = useFormSubmit("edit-bucket")

  useEffect(() => {
    if (isSuccess) onSuccess()
  }, [isSuccess, onSuccess])

  return (
    <Form name="edit-bucket" action={`/buckets/${bucket.id}`} method="patch">
      {/* form stuff */}
    </Form>
  )
}

With the callback, I don't need a new component. I can write the form inline, and pass the close callback to onSuccess:

export function EditBucketButton({ bucket }) {
  return (
    <Modal
      title="edit bucket"
      renderContent={({ close }) => (
        <Form name="edit-bucket" action={`/buckets/${bucket.id}`} method="patch" onSuccess={close}>
          {/* form stuff */}
        </Form>
      )}
    />
  )
}

Summary: the callbacks seem valuable as a convenient way to run side effects in response to a form submission.

In this specific case, I could refactor Modal to just accept props, then I could have all of the state + the effect in one component, but writing an effect for each instance of this still doesn't sound like fun 😅 That, and there are other APIs that work like this, as a point of reference.

I think one way we could support this would be to return an array from usePendingFormSubmit that has all of the states for the forms... that might be breaking. If that matters, we could make a new hook to return the array, then have the current one return the latest form, like it already does.

On another note, with the growing complexity of this, it might be worth looking into using RxJS, or some other implementation of streams/observables to handle this a little more nicely

Hi everyone!

If your company is using next-runtime in production, please let us know who you are, so we can mention you in the new README and forthcoming website.

Instructions:

• Share your project/company name
• Share your logo (in which case it might be used in the readme / frontpage etc)
• If your project is open source or public, please share a link!

dependency busboy <=0.3.1 depend on dicer
which has vunlunbility, is it possible to upgrade, I saw 1.6 available

dicer  *
Severity: high
Crash in HeaderParser in dicer - https://github.com/advisories/GHSA-wm7h-9275-46v2

Hi @smeijer
Is this ready for production?

Can I use the getServerSideProps in per component basis or has to be at page level?

How well does it handle fetch errors?

Maybe would be good to open a Discussions feature so we can talk outside of the Issues tab.

I'm interested in implementing this library into all my web apps. I was looking into Remix but it isn't stable enough for me as a solo-developer.

btw if this is a serious project I'm also willing to sponsor

I need to be able to apply className and action in some cases; it'd be convenient for the form to accept all the props that a form can accept.

A 200 status for all responses works, but being able to give a 4xx for bad responses would definitely feel more proper

Add support for <Form reloadDocument

If true, it will submit the form with the browser instead of JavaScript, even if JavaScript is on the page.

https://remix.run/docs/en/v1/api/remix#form-reloaddocument

Hello,

preview seems missing in context type.

Right now, we need to get cookies from context.req and set cookies on context.res. Although that is correct, adding cookies to the context as well, might result in a nicer api.

It doesn't take much extra code (probably ~ 5 lines including typescript), and it doesn't take any additional dependencies. So it would be in addition to what already exist.

Current:

export const getServerSideProps = handle({
  async get({ req, res }) {
    req.getCookie('user');
    res.setCookie('user', 'smeijer');
  },
});

Could then also be done like

export const getServerSideProps = handle({
  async get({ cookies }) {
    cookies.get('user');
    cookies.set('user', 'smeijer');
  },
});

Thanks for a great library! Really love this!

After updating to Next12, using the Form component, which sets the header: "Accept": "application/json", this request are just pending. using the normal HTML "form" element still works.
If you make a post request, without the "accept" header, the text/html is returned with the expected data from the handler in the next/script at the bottom.

I have created this repository to help reproduce the error: https://github.com/AndersGerner/Next12RuntimeFormError

I have tried to google and look around but I am unfortunately not able to help - Hoping you guys have an idea on this issue :)

UPDATE:
Maybe not the Form that is the problem. Using the example CURL request: curl -H "Accept: application/json" https://example.com/page-path is also pending. So maybe it's the handler?

Just a random thought, but next has currently not really a convinient way to support middleware. We could change that, and support middleware as option to handle.

A rough sketch:

handle({
  use: [currentUser, cors],
  get() { ... },
})

The signature would be compatible with Express, so that most existing middlewares could be used.

Middlewares should be able to return and throw responses.

• Thrown responses are final.

• Returned responses are merged with other return values if they hold props, or final if it's a not Found or redirect.