vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.83k stars 26.96k forks source link

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

Closed tomaszrondio closed 4 years ago

tomaszrondio commented 4 years ago

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

Expected behavior

Article.json context should not contain amp: 1

System information

Additional context

this may be connected: #17245

tomaszrondio commented 4 years ago

17461 does not fix this problem

tomaszrondio commented 4 years ago

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

yassinebridi commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

tomaszrondio commented 4 years ago

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 commented 2 years ago

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.