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.

Sign up for GitHub

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

Jump to bottom

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

Open
wawoon opened this issue Jun 17, 2020 · 7 comments
Open

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

wawoon opened this issue Jun 17, 2020 · 7 comments
Assignees
@ijjk
Labels
Pages Router Related to Pages Router.
Milestone
backlog

Comments

@wawoon
Copy link

wawoon commented Jun 17, 2020 •
edited
Loading

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.

スクリーンショット 2020-06-17 14 21 57

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
@wawoon
Copy link
Author

wawoon commented Jun 17, 2020 •
edited
Loading

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;

@timneutkens timneutkens added good first issue Easy to fix issues, good for newcomers kind: bug labels Jun 17, 2020
@timneutkens timneutkens added this to the backlog milestone Jun 17, 2020
@jonathanurias96
Copy link

Hello! Can I work in this issue?

@trulite
Copy link

trulite commented Aug 5, 2020 •
edited
Loading

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
Copy link

trulite commented Aug 6, 2020

Would love to take this up . Looking for some
Pointers

@Timer
Copy link
Member

Timer commented Aug 10, 2020

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

@Timer Timer modified the milestones: backlog, 9.x.x Aug 10, 2020
@Timer Timer added point: 2 and removed good first issue Easy to fix issues, good for newcomers labels Aug 24, 2020
@Timer Timer modified the milestones: 9.x.x, iteration 8 Aug 24, 2020
@Timer Timer modified the milestones: iteration 8, iteration 9, 9.x.x Sep 8, 2020
@yassinebridi
Copy link

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

@Timer Timer modified the milestones: 10.x.x, backlog Jan 6, 2021
@JLucasCAmorim

This comment was marked as off-topic.

Sign in to view
@balazsorban44 balazsorban44 added Pages Router Related to Pages Router. and removed area: AMP labels Apr 17, 2024
@samcx samcx removed the kind: bug label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ijjk ijjk

Labels
Pages Router Related to Pages Router.
Projects
None yet
Milestone
backlog
Development

No branches or pull requests
10 participants
@trulite @Timer @timneutkens @balazsorban44 @yassinebridi @ijjk @JLucasCAmorim @samcx @wawoon @jonathanurias96