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 and NextJS #3889

Closed
jantimon opened this issue Apr 28, 2022 · 13 comments
Closed

Relay and NextJS #3889

jantimon opened this issue Apr 28, 2022 · 13 comments

Comments

@jantimon
Copy link

Maybe you saw it already that every NextJS project ships now with Relay support by default!

https://nextjs.org/blog/next-12-1
Blog_-_Next_js_12_1___Next_js

Basically it means that the swc version of the babel-plugin-relay is built in and can be configured directly in the next.js config:

// next.config.js
module.exports = {
  compiler: {
    relay: {
      // This should match relay.config.js
      src: './',
      artifactDirectory: './__generated__',
      language: 'typescript',
    },
  },
}

NextJS does not provide a concept for splitting data loading into components. (beside from waterfall loading)
For me this makes Relay a perfect match for many medium-sized to large NextJS websites and applications.

NextJS got incredibly popular for its simplicity - unfortunately setting up Relay for NextJS is really hard.

It has been tried several times but most tries got abandoned after the POC phase:

I believe this could be done better and I believe that if someone from the Relay team could invest time here to connect the power of Relays typesafe modular data loading with NextJs it might be a big push for Relays popularity and could help the entire ecosystem.

@daniloab
Copy link
Contributor

daniloab commented Apr 28, 2022

Relay team makes an example here using next and relay https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies

I'm basing myself on this to implement in my own project with nextjs. It would be nice to have more examples besides the preloaded queries

@jantimon
Copy link
Author

Thanks that's a great start 👍

Unfortunately similar like the previous solutions it also looks like a POC which is still WIP with open TODOs but still I like the simplicity of the SSR <-> Hydration connection:
https://github.com/relayjs/relay-examples/blob/main/data-driven-dependencies/lib/relay/app.js

Especially the page part is very explicit and gives full control over which parts will be preloaded:
https://github.com/relayjs/relay-examples/blob/main/data-driven-dependencies/pages/post/%5Bid%5D.js

Maybe some parts of the relay helper could be published as a stand alone nextjs bridge.

Code like environment.getNetwork().responseCache = network.responseCache; might be hard to maintain for many teams as most devs don't know the relay internals well enough to understand what this is doing or why it is necessary.

Most teams of larger NextApps use TypeScript so I guess it would also be nice to have a clean NextJs + Relay + Typescript example - maybe right inside https://github.com/vercel/next.js/tree/canary/examples/

The part about data driven lazy loaded dependencies is mind blowing but maybe a little bit too much for a getting started example

@voideanvalue
Copy link
Contributor

Yeah I think there's not many good examples of how to use relay with nextjs. @alunyov and I created https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies in order to make some progress towards changing that. Well... that partly started as an attempt to make data-driven-dependencies, an advanced feature of relay we use at Meta internally, work in open source... I agree that there's room for simpler examples to exist.

It would make it really happy if we had more examples and documentation for using relay with nextjs (and perhaps other popular React app framework / setups too). Unfortunately creating such examples isn't a priority for the relay team at present. That said, if anyone is interested to help with creating examples and/or improving docs... please let us know... I'm interested in collaborating and supporting others in making such contributions... and perhaps some other members of the relay team might be as well!

@jantimon
Copy link
Author

jantimon commented May 6, 2022

I tried to extract the core from https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies.

The first draft is here https://github.com/jantimon/next-relay-demo

It is using React 18 + Relay + Typescript + NextJs

Some parts of data-driven-dependencies caused typescript errors:

https://github.com/jantimon/next-relay-demo/blob/682de424013437dfb20b5fbd67b51e98f175d627/data/ReactRelayContainer.tsx#L35-L36

It also has the todo from data-driven-dependencies:

https://github.com/jantimon/next-relay-demo/blob/682de424013437dfb20b5fbd67b51e98f175d627/data/ReactRelayContainer.tsx#L37

One big question is the combination of the relay haste naming convention and the next folder based router.
It seems like it would require more flexibility on the relay compiler side:

pages/pasta.tsx will require the query to be called pasta_Query
pages/Pasta.tsx will require the query to be called Pasta_Query
pages/pasta/[id].tsx will require the query to be called Id_Query
pages/tomato/[id].tsx will also require the query to be called Id_Query

Overall I really like the approach you chose for https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies as it allows to combine relay with SSG / SSR and client site data fetching 👍

@voideanvalue
Copy link
Contributor

I tried to extract the core from https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies.

The first draft is here https://github.com/jantimon/next-relay-demo

Nice! I'll try to take a closer look at some point. I haven't used TypeScript much and want to play with it more (actually I'd attempted to introduce typescript into relay-examples/data-driven-dependencies but abandoned it after silencing a lot of errors).

Some parts of data-driven-dependencies caused typescript errors:

https://github.com/jantimon/next-relay-demo/blob/682de424013437dfb20b5fbd67b51e98f175d627/data/ReactRelayContainer.tsx#L35-L36

Ah, yeah. The response cache is handy for enabling preloading of results but not well integrated in the public API.

https://github.com/relayjs/relay-examples/blob/3499f7939e68a2a3a67d6d01cfa7459de43e89ff/data-driven-dependencies/lib/relay/environment.js#L31 (i.e. what you have at https://github.com/jantimon/next-relay-demo/blob/682de424013437dfb20b5fbd67b51e98f175d627/data/relayEnvironment.tsx#L21-L22)

I did it this way because I couldn't attach the cache to the result of createNetwork because the wrapNetworkWithLogObserver call in the Environment constructor wouldn't have forwarded it. That's easily addressable.

this._network = wrapNetworkWithLogObserver(this, config.network);

But then accessing it later using environment.getNetwork().responseCache, we'd probably also need to change INetwork and IEnvironment to be parameterized types so the the type system knows there's more than execute available on the network object.

It also has the todo from data-driven-dependencies:

https://github.com/jantimon/next-relay-demo/blob/682de424013437dfb20b5fbd67b51e98f175d627/data/ReactRelayContainer.tsx#L37

iirc I left the todo in there because I want to rethink loadQuery + usePreloadedQuery (usePreloadedQuery expects a value of the return type of loadQuery... but I couldn't use loadQuery there as relay currently warns/prohibits against using it during render phase... rightly so, but that makes implementing preloading awkward).

I was thinking that instead of a response cache that uses (query id + variables) as cache key + ttl, we could make it so that there's the server can construct a preloadedQueryRef to the client (so it'd have to be serializable). It can flow into usePreloadedQuery and also into some new public API for resolving the "preloaded promise". That way, responseCache.set doesn't have to happen before usePreloadedQuery (i.e. we eliminate the room for race conditions). In fact, responseCache can be hidden away and consumers won't need to worry about creating one and attaching it to their network / environment impl.

One big question is the combination of the relay haste naming convention and the next folder based router. It seems like it would require more flexibility on the relay compiler side:

pages/pasta.tsx will require the query to be called pasta_Query pages/Pasta.tsx will require the query to be called Pasta_Query pages/pasta/[id].tsx will require the query to be called Id_Query pages/tomato/[id].tsx will also require the query to be called Id_Query

Yeah that's really awkward. I wanna improve that too. @tbezman is collaborating with us to make various relay features work well in non-haste environments. I don't think we've discussed making changes to the naming conventions that relay enforces (or have we, @captbaritone?)

I think it would be great if we could give control to consumers to define their own naming conventions via the config. I'm not sure what's the best way to do that though. Maybe something similar to module.name_mapper from Flow's config could work? Maybe we could allow consumers to completely opt out of enforcing any naming conventions (except for enforcing that names are globally unique for a project at build time)? Or maybe we could introduce some dynamic scripting abilities to give consumers even greater control (there'd be perf tradeoffs... but maybe that doesn't much for "small" projects... would also be nice to have that in order to provide support for things like #3899... but I'm also quite wary of opening the support dynamic scripting can of worms)?

Overall I really like the approach you chose for https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies as it allows to combine relay with SSG / SSR and client site data fetching 👍

Thanks! It took me something like 5 rewrites to figure out how to make Relay and Next's SSG and SSR play nice while keeping things relatively simple 😛. I'm quite pleased how it shaped up after all the rewrites.


Would you be interested in exploring any of the ideas I mentioned above?

@Choongkyu
Copy link

I had bumped my version of nextjs to 12.1.6 and am seeing this error when adding a tagged template literal graphql mutation: Invariant Violation: graphql: Unexpected invocation at runtime. Either the Babel transform was not set up, or it failed to identify this call site. Make sure it is being used verbatim as 'graphql'. This is for a project that was using preval and babel-plugin-macros a couple years ago. The next-relay-demo repo runs just fine for me but I was hoping that I can modify the existing project instead of having to start fresh with the next-relay-demo repo and then move my components, pages, and api route handlers on top of it. Thank you.

@petrbela
Copy link

petrbela commented May 27, 2022

@Choongkyu This is most likely because you're missing either compiler: { relay: require('./relay.config') } in next.config.js (if using SWC compiler) or babel-plugin-relay in babel.config.js (if using Babel).

@tbezman
Copy link
Contributor

tbezman commented May 27, 2022

@Choongkyu , what @petrbela sounds about right to me. If that doesn't work, maybe you can post some example minimum repro and I can check it out

@Choongkyu
Copy link

@Choongkyu This is most likely because you're missing either compiler: { relay: require('./relay.config') } in next.config.js (if using SWC compiler) or babel-plugin-relay in babel.config.js (if using Babel).

thank you. My relay.config.js looks like this:

module.exports = {
  src: "./",
  artifactDirectory: "__generated__",
  schema: "./schema.graphql",
  exclude: [
    "**/node_modules/**",
    "**/.next/**",
    "**/__mocks__/**",
    "**/__generated__/**",
  ],
  language: "typescript",
};

I'm not exactly sure how to repro the issue but walking away from it and returning to it somehow resolved it. Thank you.

@jantimon
Copy link
Author

@tbezman @Choongkyu NextJs switched from BabelJS to SWC. The build-in Relay support in NextJs is only for SWC.

This issue is about finding the best integration between the latest NextJs and Relay versions.

If you are still using BabelJs this solution might not work for you.
Could you please move problems with BabelJs to another thread?

@voideanvalue

Would you be interested in exploring any of the ideas I mentioned above?

I would love to work further on the integration but I would need some guidance.
What would be the most important part I should start with?

@punkpeye
Copy link

Is there a more up to date version of an example for integration Next.js with Relay?

@soneymathew
Copy link
Contributor

@punkpeye https://github.com/soneymathew/relay-3d-example is a WIP that may be of some help.
it uses the latest versions as of today.

https://github.com/soneymathew/relay-3d-example/blob/07a1d5ce4df6dad251ab2185df2cf8a442d9806d/directories-3d/package.json#L23

https://github.com/soneymathew/relay-3d-example/blob/07a1d5ce4df6dad251ab2185df2cf8a442d9806d/directories-3d/package.json#L27-L28


The part about data driven lazy loaded dependencies is mind blowing but maybe a little bit too much for a getting started example

I agree @jantimon 3D is pretty exciting, this example showcases 3D feature of relay client


Used https://github.com/relayjs/relay-examples/tree/main/data-driven-dependencies as a base for this... just barely managed to convert it all to Typescript 😅

@jantimon
Copy link
Author

I guess most of this discussion is outdated now that Next.JS 13 has been released.
Therefore I opened a new issue: #4107 and will close this issue.

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

8 participants