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

AMP: hidden attribute on amp elements causes a validation error #8861

Closed
sebastianbenz opened this issue Sep 26, 2019 · 6 comments
Closed

AMP: hidden attribute on amp elements causes a validation error #8861

sebastianbenz opened this issue Sep 26, 2019 · 6 comments

Comments

@sebastianbenz
Copy link
Contributor

sebastianbenz commented Sep 26, 2019

Bug report

Describe the bug

Using AMP's built-in hidden attribute on an AMP element results in a validation error:

error  The attribute 'hidden' in tag 'amp-img' is set to the invalid value 'true'.  https://amp.dev/documentation/components/amp-img

The problem is that hidden is converted into hidden=true which causes the validation error above. Using hidden on a non-AMP element works though (e.g. <div hidden>).

To Reproduce

This will cause a validation error:

import Head from 'next/head';

export const config = { amp: true };

const Index = () => (
  <amp-img hidden src="https://unsplash.it/300/200" width="300" height="200"></amp-img>
  <div hidden>Test</div>
);

export default Index;

Expected behavior

The hidden attribute should be supported on AMP elements.

System information

  • Version of Next.js: 9.0.6
@c0b41
Copy link
Contributor

c0b41 commented Sep 26, 2019

How about instead empty string?

import Head from 'next/head';

export const config = { amp: true };

const Index = () => (
  <amp-img hidden="" src="https://unsplash.it/300/200" width="300" height="200"></amp-img>
  <div hidden>Test</div>
);

export default Index;

@sebastianbenz
Copy link
Contributor Author

@c0b41 thanks, that's a good workaround! However, I find the inconsistent serialization behavior confusing.

@Timer Timer added this to the 9.0.x milestone Sep 26, 2019
@Timer
Copy link
Member

Timer commented Sep 28, 2019

Note this semi-conflicts with the React team's plan with the hidden attribute.

facebook/react#16831
https://www.youtube.com/watch?v=6g3g0Q_XVb4&feature=youtu.be&t=1650

@Timer
Copy link
Member

Timer commented Sep 28, 2019

I'm not sure this is something Next.js should really try to "patch", as its just a gotcha with JSX.

@sebastianbenz
Copy link
Contributor Author

I agree. Just found the related react issue.

@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 31, 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

4 participants