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

SSG AMP hybrid mode dynamic route adds "amp":1 to context on build in non AMP article.json #18128

Closed
tomaszrondio opened this issue Oct 22, 2020 · 7 comments

Comments

@tomaszrondio
Copy link

Bug report

Describe the bug

When using a dynamic route in hybrid mode with getStaticProps and getStaticPaths, all static generated files have added amp: 1 to context.

E.g if getStaticPaths returns /article for [slug].js then these files are generated:

article.amp.html
article.amp.json
article.html
article.json

and in article.json context is

context: { 
  params: {
    slug : ["article"],
    amp: 1
  }
}

So first visit on article loads AMP component . Everything goes back to normal once revalidated.

To Reproduce

https://github.com/SuperdeskWebPublisher/publisher-pwa

  • download repository.
  • build
  • go to .next/server/pages/business/WHATEVER.json and look for context (value of context.params is added to pageProps)

Expected behavior

Article.json context should not contain amp: 1

System information

  • OS: macOS
  • Version of Next.js: 9.5.5
  • Version of Node.js: 10.15.0

Additional context

this may be connected: #17245

@tomaszrondio
Copy link
Author

#17461 does not fix this problem

@tomaszrondio
Copy link
Author

I moved my logic that was based on context to useAmp hook and it fixed the issue.

@yassinebridi
Copy link

I moved my logic that was based on context to useAmp hook and it fixed the issue.

Can you please share a snippet of how you did that.

@tomaszrondio
Copy link
Author

I was having logic in getStaticProps(context) like that const _isAmp = context.params.amp ? true : false; and simply moved it directly to page component. Here is my commit https://github.com/SuperdeskWebPublisher/publisher-pwa/commit/98862e635678178a66f481e80849388dd12a707f#diff-64730026d6dc32e1eedd945497d765a29d0939e3845877b9dd7a685059b026d0

@yassinebridi
Copy link

Oh yeah, that's actually the recommended way to do it.

@tomaszrondio
Copy link
Author

I believe that sooner or later there will be a case when someone will really need to know if isAmp at the level of getStaticProps. But yeah so far so good :)

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants