wawoon commented 4 years ago

Bug report

Describe the bug

When SSG with fallback: true and amp: "hybrid" are used together, the paths not specified in getStaticPaths have an invalid href in <link rel="amphtml"> tag.
- For example, when there is a pages/[slug].jsx and the user visits /bar, the <link rel="amphtml"> tag will refer to /[slug].amp.

To Reproduce

https://github.com/wawoon/next-amp-ssg-fallback-reproduce

pages/[slug]/index.jsx

export default (props) => {
  return <div>slug: {props.slug}</div>;
};

export const config = {
  amp: "hybrid",
};

export const getStaticProps = async (ctx) => {
  return {
    props: {
      slug: ctx.params.slug,
    },
  };
};

export const getStaticPaths = async () => {
  return { paths: [{ params: { slug: "foo" } }], fallback: true };
};

※ caution: When you add unstable_revalidate to getStaticProps, the href of <link rel="amphtml"> is overwritten while regeneration by #14251

When the user visits http://localhost:3000/bar, bar is not specified in getStaticPaths, the href of <link rel="amphtml"> refers to /[slug].amp. And this /[slug].amp is invalid url.

Expected behavior

The /bar should have correct amp page path, even when the path is not included in getStaticPaths.
The amp version of /bar should be generated using getStaticProps dynamically.

Screenshots

System information

macOS
Chrome
Next: 9.4.5-canary.12
Node: v12.14.1

Additional context

I have recently created #14251 which is related with amp page with SSG.

wawoon commented 4 years ago

This is a memo to workaround the issue:

IMO, we should avoid using { amp: "hybrid" } and getStaticProps together. It is too complicated to use {amp: "hybrid"} and other options.

Instead of using { amp: "hybrid" }, we should create /[slug]/index.jsx and /[slug]/amp.jsx separately, and set <link rel="amphtml"> for ourselves. I confirmed the Incremental Static Regeneration works with AMP.

like:

pages/[slug]/index.jsx

import { useAmp } from "next/amp";
import Head from "next/head";

export default (props) => {
  const isAmp = useAmp();
  return (
    <div>
      <Head>
        <link rel="canonical" href={`https://example.com/${props.slug}`} />
        {!isAmp && (
          <link rel="amphtml" href={`https://example.com/${props.slug}/amp`} />
        )}
      </Head>
      {props.slug}
    </div>
  );
};

export const getStaticProps = async (ctx) => {
  return {
    props: {
      slug: ctx.params.slug,
    },
  };
};

export const getStaticPaths = async () => {
  return { paths: [{ params: { slug: "foo" } }], fallback: true };
};

pages/[slug]/amp.jsx

import Original from "./index";
export { getStaticPaths, getStaticProps } from "./index";

export const config = { amp: true };
export default Original;

jonathanurias96 commented 4 years ago

Hello! Can I work in this issue?

trulite commented 4 years ago

I followed the workaround suggested but could not get AMP to load the page dynamically.

I used

//Checking for fallback const router = useRouter(); if (router.isFallback) { return <div>Loading...</div> } //Is stuck at loading... If I get the amp version it is always stuck at Loading... ; the normal page loads correctly

trulite commented 4 years ago

Would love to take this up . Looking for some Pointers

Timer commented 4 years ago

AMP will require you use fallback: 'unstable_blocking', maybe we can toggle this by default for AMP pages!

yassinebridi commented 4 years ago

@Timer I'm using fallback: 'blocking', with Next@10, still has the same problem, it doesn't revalidate.

JLucasCAmorim commented 2 years ago

Hi guys, do we have any updates about it?

vercel / next.js

SSG with fallback doesn't generate AMP page dynamically #14256