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

Relay example with Next.js v13 #241

Merged
merged 9 commits into from
Dec 5, 2022
Merged

Relay example with Next.js v13 #241

merged 9 commits into from
Dec 5, 2022

Conversation

alunyov
Copy link
Contributor

@alunyov alunyov commented Nov 8, 2022

This example is a simplified version of the Issue Tracker Example, and it demonstrates a possible Relay integration with the Next.js v13 and React Server Components.

Additionally, this example is using parts of the data-driven-dependecies example, mainly to setup Next.js integration. However, the code that is related to the server preloading is much simpler now.

cc @voideanvalue I would love your feedback on this, as you've been actively looking into these.

Copy link

@noghartt noghartt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that don't need to commit __generated__ folder.

Copy link

@vit0rr vit0rr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove __generated__ folder?

Copy link

@Amorim33 Amorim33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!
Is it possible to use Turbopack?

@alunyov
Copy link
Contributor Author

alunyov commented Nov 8, 2022

Cool! Is it possible to use Turbopack?

It should be possible

@araujobarret
Copy link

araujobarret commented Nov 17, 2022

Thanks for creating this demo @alunyov!

I've tried to run your branch and also a project from my work with NextJS 13 and Relay and I'm getting the following error:

$ mkdir -p __generated__ && concurrently "relay-compiler --watch" "next dev"
[0] [INFO] querying files to compile...
[1] warn  - Port 3000 is in use, trying 3001 instead.
[1] ready - started server on 0.0.0.0:3001, url: http://localhost:3001
[0] [INFO] [my_app] compiling...
[0] [INFO] [my_app] compiled documents: 4 reader, 3 normalization, 5 operation text
[0] [INFO] Compilation completed.
[0] [INFO] Watching for new changes...
[1] warn  - You have enabled experimental feature (appDir) in next.config.js.
[1] warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.
[1] 
[1] info  - Thank you for testing `appDir` please leave your feedback at https://nextjs.link/app-feedback
[1] thread 'thread 'thread 'thread '<unnamed><unnamed><unnamed>' panicked at '<unnamed>' panicked at '' panicked at 'pages_dir is expected.pages_dir is expected.pages_dir is expected.', ' panicked at 'pages_dir is expected.crates/core/src/relay.rs', ', crates/core/src/relay.rs', :crates/core/src/relay.rs::180180:crates/core/src/relay.rs:48:180
[1] 48note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[1] :180
[1] :48
[1] 48
[1] thread '<unnamed>' panicked at 'pages_dir is expected.', crates/core/src/relay.rs:180:48
[1] thread '<unnamed>' panicked at 'pages_dir is expected.', crates/core/src/relay.rs:180:48
[1] thread '<unnamed>' panicked at 'pages_dir is expected.', crates/core/src/relay.rs:180:48
[1] thread '<unnamed>' panicked at 'pages_dir is expected.', crates/core/src/relay.rs:180:48
[1] thread '<unnamed>' panicked at 'pages_dir is expected.', crates/core/src/relay.rs:180:48
[1] error - ./node_modules/next/dist/client/app-next-dev.js
[1] Error: failed to process

I'm using Node v16.8.0

@alunyov
Copy link
Contributor Author

alunyov commented Nov 17, 2022

I see, @araujobarret - I need to update the PR to include pages. It seems like Next.js doesn't like when the pages dir is missing.

To unblock yourself, just create a pages directory.

@araujobarret
Copy link

araujobarret commented Nov 17, 2022

I see, thanks for the fast reply!

I was wondering if it was necessary since the new appDir stuff doesn't require /pages anymore since all pages would be under /app.

ijjk pushed a commit to vercel/next.js that referenced this pull request Nov 20, 2022
Fixes:
relayjs/relay-examples#241 (comment)

**Context:**
Current relay transformer requires `pages_dir` in its entry point:

https://github.com/vercel/next.js/blob/ecfd2f4cd6a1c3f6bc3cd1cca6b62a3cb705a4e2/packages/next-swc/crates/core/src/relay.rs#L184

But consider the case that `pages_dir` is not provided because the page
system is built entirely in the app directory introduced from Next.js
13.

In this case, transformer causes unconditional panics even though
there's nothing wrong with it.

This PR removes panic in entry point and changes the type of `pages_dir`
into `Option<T>` so that keeps [build stability for existing page
system](https://github.com/vercel/next.js/blob/ecfd2f4cd6a1c3f6bc3cd1cca6b62a3cb705a4e2/packages/next-swc/crates/core/src/relay.rs#L157-L160)
even if `pages_dir` is not provided.
@vinibgoulart
Copy link

do you have an idea how to create a boilerplate for reactRelayContainer? I don't know if I like the idea of having to define an environment and container for each page

wouldn't it be ideal to define a container in the layout next feature? and use for the whole application?

maybe adapt it https://github.com/sibelius/relay-workshop/blob/main/apps/next-app/src/relay/ReactRelayContainer.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants