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

Does not work correctly when Next.js config output option is set to 'standalone' #99

Closed
Jon1VK opened this issue Jun 14, 2023 · 10 comments

Comments

@Jon1VK
Copy link
Contributor

Jon1VK commented Jun 14, 2023

When running Next.js as a standalone server for example in EC2 or docker container, the Router functionality breaks.

Since Next.js is ran as a basic Node.js server in a standalone mode, the Router class is shared for each page generation. As so, the Router.getPageHref() can return wrong values in the generation phase of the page.

Here is an example, what goes wrong:

1. Page "/fi" gets revalidated
2. Page "/fi" regenaration is started
3. Router.setPageHref("/fi") is called
4. Page "/en" gets revalidated
5. Page "/en" regenaration is started
6. Router.setPageHref("/en") is called
7. Page "/fi" calls Router.getPageHref() --> returns wrong value "/en" for "/fi" page
8. Page "/fi" regeneration is finished with wrong data
9. Page "/en" regeneration is finished with correct data

Problem can be solved by removing Router.setPageHref() and Router.getPageHref() and by passing pageHref always to the OriginPage straight through props and modifying functions that use Router.getPageHref to take the pageHref as a parameter.

@Jon1VK Jon1VK changed the title Does not work correctly when Next.js output: option is set to 'standalone' Does not work correctly when Next.js config output option is set to 'standalone' Jun 14, 2023
@svobik7
Copy link
Owner

svobik7 commented Jun 14, 2023

Actually I was hesitating when I introduced this feature of Router.getPageHref. It was introduced just to help developers to get current page href down in the code. But as you wrote when Next.js is in non-serverless mode it does not work.

I agree with you that this setPageHref and getPageHref can be dropped and actual pageHref string be passed to page directly.

I will update the package 🙂

@Jon1VK
Copy link
Contributor Author

Jon1VK commented Jun 14, 2023

Sadly the Context API cannot be used in server components to pass the pageHref value further down for other components. Props drilling is currently the only option, or at least I cannot think of another way. 😞

@Jon1VK
Copy link
Contributor Author

Jon1VK commented Jun 15, 2023

It would be also nice, if in addition of the pageHref also the locale would be passed down to the page.tsx components. Just like in Layouts locale is passed to OriginLayout.

Since the Router.getPageHref() cannot be used now, one has to extract manually the locale from the pageHref everytime in PageOrigin component. And developers probably more often have a need to use the locale and not the pageHref 🤔

Should I create another issue about this idea?

@svobik7
Copy link
Owner

svobik7 commented Jun 15, 2023

It would be also nice, if in addition of the pageHref also the locale would be passed down to the page.tsx components. Just like in Layouts locale is passed to OriginLayout.

This is not needed IMO. There is this Router.getLocaleFromHref method that can suite that use case well.

@svobik7
Copy link
Owner

svobik7 commented Jun 15, 2023

Second thought about removing setPageHref and getPageHref brought me to the conclusion that these methods are essential for doing this (copied from Readme):

Passing the locale parameter is not required. If you do not pass any locale param then the current page locale will be automatically used.

// on "/cs" page it will creates "/cs/o-nas" href while on "/" (en) it will create "/about" href
router.getHref('/about')

I am thinking now how we can isolate the class instance to just one node process/request.

@svobik7
Copy link
Owner

svobik7 commented Jun 15, 2023

One of the solution here could be introducing just tiny RouterContext as client component that will wrap the whole page and will provide just currentLocale and currentPageHref that can be consumed down in the tree.

const {locale, pageHref} = useContext(RouterContext)
router.getHref("/about", {locale})

But I don't like it much as it is not Pure Server.

@Jon1VK
Copy link
Contributor Author

Jon1VK commented Jun 15, 2023

Maybe the Router.setPageHref and Router.getPageHref can be left as it is but just add a notice that in standalone mode locale must be passed directly to getHref function 🤷 That's how I avoided the problem I described when in standalone mode.

That way no changes are necessary.

@Jon1VK
Copy link
Contributor Author

Jon1VK commented Jun 15, 2023

Yeah, the context is not a nice option since it's client side code and can be used only in client side components.

@Jon1VK
Copy link
Contributor Author

Jon1VK commented Jun 15, 2023

Maybe to be sure, you could just change it so that the pageHref is passed directly to the page and other templates where pageHref was passed. So something like this:

export const tplStatic = `
import ${PATTERNS.originName}Origin from '${PATTERNS.originPath}'
import { Router } from 'next-roots'

export default function ${PATTERNS.originName}(props:any) {
  Router.setPageHref("${PATTERNS.pageHref}")
  {/* @ts-ignore */}
  return <${PATTERNS.originName}Origin {...props} pageHref="${PATTERNS.pageHref}" />
}
`

I don't think this actually really matters since code here is always synchronous and I don't think there is a possibility that pageHref could change between the two Router.set and get calls that were before in the above template.

So from my perspective this issue can be closed as I found already a workaround for this problem

@svobik7
Copy link
Owner

svobik7 commented Jun 15, 2023

Agreed. I updated Readme file with our using getHref in standalone mode. Thank you for spotting this 👍

@svobik7 svobik7 closed this as completed Jun 15, 2023
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

2 participants