feature: set runtime config (support next/config)

[alexandermendes]

Hi, this looks like a really useful library so far! I just came across this while trying it out for the first time.

What kind of change does this PR introduce?

Sets the Next.js runtime config, so we can test pages that make use of next/config.

What is the current behaviour?

The runtime config is not set.

What is the new behaviour?

The runtime config is set!

Does this PR introduce a breaking change?

Nope.

Other information:

Please check if the PR fulfills these requirements:

• Tests for the changes have been added
• Docs have been added / updated

[coveralls]

Coverage remained the same at 100.0% when pulling c55804d on alexandermendes:runtime-config into 5adeec7 on toomuchdesign:master.

[Meemaw]

Thanks for the PR @alexandermendes, looks good! 🚀

I have one request though: if I'm not mistaken, serverRuntimeConfig should only be accessible on server while publicRuntimeConfig should be accessible on both, server and client. Could you add some tests that ensure this is the case? We have a lot of test examples that are using getServerSideProps / getInitialProps you can look at.

[Meemaw]

It's a bit confusing that getter mutates some "global" state (has side effects). Could we pull that out and let the callee execute it?

[alexandermendes]

Yeah good point, was avoiding moving too much around but agree it's confusing, updating!

[alexandermendes]

It's not immediately obvious where the best place might be to do this 🤔 There are those executeAsIfOnServer() and executeAsIfOnServerSync() functions where we could set the server-side variant (and just set the public variant higher up somewhere) but that means we have to start passing the next config, or nextRoot setting around more. We have this instance in initTestHelpers() where we won't have access to the nextRoot anyway.

I have pushed up an idea there, but it's not ideal as calling getConfig() on the server will still return just the publicRuntimeConfig, it's obviously only when we access serverRuntimeConfig that it will be returned. Any other ideas about where this kind of differing client/server functionality might be added?

[toomuchdesign]

Hi @alexandermendes, thanks for joining us!
I've never used next/config, but it seems to be a sort of singleton, right?

We're in the usual trouble of simulating server and client environments at the same time, we're definitely finding our patterns :)

Would it be feasible switching next/config between server and client mode when needed? Like client mode by default + turning on server mode while fetching data?

Extra considerations

Given the example provided by docs, getConfig() result might be stored at module level, causing the same issue as #64 and #67.

As long as we don't find a way of properly separating server and client environments we might not be able to fully support next/config, then.

Would it be an acceptable first step implementing a partial next/config which only returns publicRuntimeConfig? This might cover a part of users' needs and pave the way for a future full implementation.

Happy to hear your thoughts. 🙌

[alexandermendes]

Hi, yeah I haven't looked too deeply, but I think next/config is just this and that's set differently on the client and server (with a few other variants for things like statically exported pages). As you say, probably a wider issue to figure out really.

Just implementing publicRuntimeConfig for now is a decent idea. Having it partly implemented could also be confusing though. I see you're looking at this client/server stuff already in #73 so maybe we can just leave this here for a bit (or close) and see if it becomes any easier when that stuff has been figured out 😄

[toomuchdesign]

Hi @alexandermendes, it seems we might be able to ship a solution for client/server environment sooner or later. Would you like to get back on this when we'll be able to merge #73?

PS. You might also rebase over #73 and change PR branch target to try and see whether this could work with the new client/server approach.

[toomuchdesign]

Hi @alexandermendes! #73 has been merged, and now master should have enough surface area to hook in next/config in both client and server mode. I'll be happy to support if you still have the opportunity to work on your PR.

At a first sight, a plausible solution might consist of updating global next/config output to switch between server and client mode based on the operation we're about to perform.

An optimistic approach might consider doing this switch directly in src/server.ts's executeAsIfOnServer function, but this definitely remains to be seen.

Given the current complexity of these client + server states I'd suggest to fully cover the feature with tests and then try out a working implementation.

Cheers (and thanks)!

[toomuchdesign]

With #90 being merged, we're in an ideal position to add next/config support. @alexandermendes are you still able to work on this or you'd prefer someone taking over? Have a bright day 🙌

[toomuchdesign]

Closed in favor of #102.