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

[lucia] Attempt to switch to nextjs app dir #139

Open
wants to merge 4 commits into
base: lucia
Choose a base branch
from

Conversation

rmarscher
Copy link
Contributor

@rmarscher rmarscher commented Jan 5, 2024

This does seem to get styles working with SSR pages. It's a bit unclear if tRPC is waiting for queries to run during SSR. They say they don't have a way to hydrate those queries even if they do run during SSR, so it's a waste. trpc/trpc#3185 (comment)

They have different clients for server-side rendering including one that uses an app-dir hosted trpc api and bypasses internal http requests (which is how things work in sveltekit land with their endpoints). Their client for fetching in the browser is marked as "not implemented" in their example repo:
https://github.com/trpc/trpc/blob/main/examples/.experimental/next-app-dir/src/app/client/ClientGreeting.tsx#L11

I created a custom provider with TRPCReact which is how I've seen others integrate tRPC with NextJS app dir (the TRPCNext withTRPC HOC only works with the old Pages router).

@rmarscher rmarscher force-pushed the next-app-dir-experimental branch 3 times, most recently from 0720e7c to 0bdf789 Compare January 5, 2024 22:31
@rmarscher
Copy link
Contributor Author

I deployed it to here. https://t4-rm-test.pages.dev/ssr

@rmarscher rmarscher marked this pull request as ready for review January 5, 2024 22:34
@rmarscher
Copy link
Contributor Author

We could rework this PR to be against master and then merge it to lucia and make additional adjustments so it makes it to both places. This incorporates the same changes that are in #138 in addition to the app dir update.

@timothymiller
Copy link
Owner

To be clear, styles don't load properly in the old pages method when getServerSideProps runs in the route?

It does work in the app router?

@timothymiller
Copy link
Owner

I want to investigate this some more before merging it into the lucia branch. I feel like it could be it's own branch?

The app router directives in app/features probably breaks expo compatibility fwiw.

@rmarscher
Copy link
Contributor Author

rmarscher commented Jan 6, 2024

I want to investigate this some more before merging it into the lucia branch. I feel like it could be it's own branch?

Yeah... probably shouldn't merge for a bit or use a different branch.

The app router directives in app/features probably breaks expo compatibility fwiw.

I just tested with the iOS simulator and everything was fine. It redirected me without issue. It's using Solito's app router hooks - https://solito.dev/app-directory/hooks

I see warnings in my nextjs console about StyleSheet.compose(a, b) is deprecated; use array syntax, i.e., [a,b]. - I'm guessing it's something internal with the way they say to handle CSS-In-JS libs like Tamagui. See the styles-provider.tsx component https://github.com/timothymiller/t4-app/pull/139/files#diff-bace245944bab2252f90569d5c47e6e2a5fe9b067a49d62bf47437d64061c866

I also see a jotai warning sometimes. Detected multiple Jotai instances. It may cause unexpected behavior with the default store. https://github.com/pmndrs/jotai/discussions/2044 It seems to just be caused be hot reloads so maybe not an issue... but with SSR, I know global vars like jotai uses can be problems. The author says to use a Provider with NextJS so I think that is probably the correct path. pmndrs/jotai#2044 (reply in thread)

One other note... dealing with Cloudflare's cross-domain default domains for the API (*.workers.dev) and Pages (*.pages.dev) is super annoying. I tried using SameSite=none which seemed to work in chrome but I was having trouble on iOS. Maybe it can work... but browsers seem to be locking this down more. See lucia-auth/lucia#1320 (comment)

I think the best path forward for this template is to use the NextJS rewrite feature to proxy the worker hono server from the Pages site. I've been using this for months with my personal project. It doesn't seem to add much noticeable latency. Using a custom domain and setting the worker to be on a subdomain of the app domain works though as you know... but the docs need to tell people that it's required to use a custom domain when they deploy.

Here's a commit where I switched to the proxied API on my master branch rmarscher@f08abae -- that is currently deployed at https://t4-rm-test.pages.dev/.

@rmarscher
Copy link
Contributor Author

To be clear, styles don't load properly in the old pages method when getServerSideProps runs in the route?

It does work in the app router?

Essentially, yes. When using getServerSideProps, it switches the page from a static build (i.e. no runtime) to the edge runtime. The getInitialProps function in the _document.tsx file used to add styles to the page is not supported by the edge runtime. It only works at build time or with the nodejs runtime. So the styles never get injected into the output. Here is the open bug with vercel - but I suspect it will be closed wont-fix. vercel/next.js#40440

With the app router, it uses a different method to inject the styles which does work on the edge runtime.

@zvictor
Copy link

zvictor commented May 10, 2024

There is quite a lot of work in this PR! Much appreciated!

I'm new here, just stumbled upon T4 by accident while setting up a new Web project.
After checking its features I started wondering why not build a T4 app instead of a traditional web one.

However, the lack of /app made me stop for a moment, as I wouldn't want to start something new without support to that.
With that been said, I'd still be willing to give T4 a try as an alpha tester of this PR if you think it's usuable as it is.

Is there anything special that I should know in order to have this PR up and running in my project?

@rmarscher
Copy link
Contributor Author

However, the lack of /app made me stop for a moment, as I wouldn't want to start something new without support to that. With that been said, I'd still be willing to give T4 a try as an alpha tester of this PR if you think it's usuable as it is.

As mentioned in the comments, I got hung up on tRPC. The @trpc/next package only supports the pages router. They don't have an official way to work with React Server Components and app router. I think they might be waiting for React 19 and its new "use" hook. They were using that in one of the experimental app router examples they had previously published. I just checked and there are still open issues around nextjs app router in the trpc repo.

Tamagui added basic support for app router but they are "actively working on a new mode that enables full server-side support with some limitations." https://tamagui.dev/docs/guides/next-js#app-router

The other thing is the native Expo app communicates with the tRPC API to get all of its data. Expo and the React Native community are experimenting with how RSC can be used to optimize bundles for native apps, but that feels pretty far off from being something to use in production.

That being said... this branch was working last I tried it. I don't remember there being anything special to get it working. It is missing the lucia v3.0 updates that we merged on the lucia branch though. There was a fix to how the argon2 wasm file needed to be invoked for password hashing too.

@rmarscher
Copy link
Contributor Author

rmarscher commented May 12, 2024

@zvictor I just rebased and force-pushed the branch so it has the latest commits from the lucia branch.

As I mentioned, this project is currently expecting a separate Cloudflare Worker for the API from the Cloudflare Pages project that hosts the NextJS app. With app router, it's probably possible to move the API server code to be part of the NextJS app. You'd have to use the wrangler.toml to in the apps/next folder to define the bindings that are currently set to the packages/api/wrangler.toml. Once you have the API server set up in your NextJS app router, you can use a server side react context to store a tRPC caller and then you could use that in your React Server Components to fetch data server side and send it down.

There's a talk happening this week - "Introducing Universal React Server Components in Expo Router" - https://conf.react.dev/talks/7

Expo Router v4 is probably getting released. Looking forward to seeing how that enables RSC for native.

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 this pull request may close these issues.

4 participants