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

[Next.js v9.2.2] amp-optimizer v2 makes AMP page valid automatically. #11096

Closed
yosuke-furukawa opened this issue Mar 16, 2020 · 4 comments
Closed

[Next.js v9.2.2] amp-optimizer v2 makes AMP page valid automatically. #11096

yosuke-furukawa opened this issue Mar 16, 2020 · 4 comments

Comments

@yosuke-furukawa
Copy link
Contributor

Bug report

We are using AMP page but we would not want to make our page amp-valid.
see: #9176

But amp-optimizer creates valid amp automatically, so we are on amp-cache.
We don't need the feature for our situation.

related: #10705

Describe the bug

v9.2.1 we could use invalid amp, we have upgraded to v9.2.2, our page turns into amp-valid pages.

To Reproduce

  1. create amp invalid page using <html></html>, this tag needs to have "amp" attribute to valid page, but we don't set amp attribute on the html.
  2. build our page and launch app as prod mode, we could see <html amp> tag.

Expected behavior

This feature is useful to create valid amp, so this behavior is not so weird for most developers. But we would like to use amp as UI framework. we need to set custom optimizer option.

see: #10705

Screenshots

N/A

System information

  • OS: N/A
  • Browser: N/A
  • Version of Next.js: v9.2.2
@sebastianbenz
Copy link
Contributor

This makes sense. Adding support for passing a config AMP Optimizer config is useful on any case. I'd propose adding a new experimental flag:

amp: {
  experimental: {
   optimizer: { ... },
    validator: "./validator_minified.js",
  } 
 }

@yosuke-furukawa
Copy link
Contributor Author

@sebastianbenz Thank you! I have already sent PR. if you could, PTAL the PR. #10705

@yosuke-furukawa
Copy link
Contributor Author

Thank you for merging this PR. I will close this issue.
#10705

@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 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sebastianbenz @yosuke-furukawa @balazsorban44