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 for server components #347

Closed
kachkaev opened this issue May 27, 2024 · 5 comments · Fixed by #352
Closed

Support for server components #347

kachkaev opened this issue May 27, 2024 · 5 comments · Fixed by #352

Comments

@kachkaev
Copy link
Contributor

kachkaev commented May 27, 2024

👋 @zpao! Just curious if you have any plans to support server components. I am experimenting with og images in Next.js (vercel/satori), writing something like this:

import { ImageResponse } from "next/og";
import { QRCodeSVG } from "qrcode.react";

export function GET(): Response {
  return new ImageResponse(
    (
      <div tw="flex w-full h-full items-center justify-center bg-[#cfc]">
        <QRCodeSVG
          value="https://example.com"
          width={100}
          height={100}
          includeMargin={true}
        />
      </div>
    ),
    {
      width: 200,
      height: 200,
    },
  );
}

This code works!

image

However, there are warnings during next dev, next build and next start If I add export const runtime = "edge"; to the file:

 ⚠ ./node_modules/.pnpm/[email protected][email protected]/node_modules/qrcode.react/lib/esm/index.js
Attempted import error: 'useRef' is not exported from 'react' (imported as 'useRef').

Import trace for requested module:
./node_modules/.pnpm/[email protected][email protected]/node_modules/qrcode.react/lib/esm/index.js
./app/test/route.tsx

./node_modules/.pnpm/[email protected][email protected]/node_modules/qrcode.react/lib/esm/index.js
Attempted import error: 'useEffect' is not exported from 'react' (imported as 'useEffect').

...

Regardless of Next.js intricacies, it seems that qrcode.react lacks React server component support. This line imports hooks that are not available on the server:

import React, {useRef, useEffect, useState, useCallback, useMemo} from 'react';

What do you think about splitting canvas and svg implementations into two files? If we import an SVG, it can be server-rendered. If it’s a canvas, there is "use client" on top. I reckon that both QRCodeCanvas and QRCodeSVG can be still re-exported from src/index.tsx, so the change is non-breaking.

@zpao
Copy link
Owner

zpao commented Jun 24, 2024

Nothing against it at all. IIRC the issue with splitting is that it requires restructuring the actual encoder to work with our current toolchain.

This line imports hooks that are not available on the server

I don't see any actual documentation on what can or cannot be imported in server components. I can start a next project and go through trial and error, but that's not ideal.

@kachkaev
Copy link
Contributor Author

kachkaev commented Jun 24, 2024

Cool!

I don't see any actual documentation on what can or cannot be imported in server components.

I believe that it applies to anything that’s stateful. E.g. useEffect, useState, useCallback, etc.

#188 is a step in the right direction, it’s just missing "use client" in canvas.tsx. We can push the suggestion to @caub’s PR or I can create a new PR from scratch if that’s easier. What would you prefer?

@kachkaev
Copy link
Contributor Author

kachkaev commented Jul 7, 2024

Ping @zpao 👋
Happy to create a PR if you’d prefer that over #188

@zpao
Copy link
Owner

zpao commented Jul 7, 2024

The other PR is going to need to be updated a bunch anyway so feel free to make a new one. Like I said though, splitting the components is easy enough. The tooling right now blows up generating the TS declaration once you split and that's the bigger effort.

@kachkaev
Copy link
Contributor Author

kachkaev commented Jul 8, 2024

Looks like we can get away with something even simpler: #352. Typescript tooling and file splitting can be done later (much later TBH).

@zpao zpao closed this as completed in #352 Jul 8, 2024
@zpao zpao closed this as completed in 9c269e7 Jul 8, 2024
zpao pushed a commit that referenced this issue Sep 1, 2024
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

Successfully merging a pull request may close this issue.

2 participants