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

with-amp example is not pass AMP Validator #3155

Closed
phongtattuan opened this issue Oct 23, 2017 · 9 comments
Closed

with-amp example is not pass AMP Validator #3155

phongtattuan opened this issue Oct 23, 2017 · 9 comments

Comments

@phongtattuan
Copy link

Because of AMP restriction, it does not allow custom script,
But the server rendering include custom script of next.js, so with-amp example is not pass AMP Validator.
Can we remove those scripts ?

Thank you.

@ismamz
Copy link
Contributor

ismamz commented Oct 23, 2017

There are two problems:

  1. <meta charset="utf-8" /> appears twice (solution: remove this line)

  2. The attribute 'as' may not appear in tag 'link rel='.

To remove the as attribute from Next.js scripts you can do the following:

Add a _head.js file into /pages directory (like _document.js).

In this file you should extend Head and copy 3 methods from main class:

// _head.js
import { Head } from 'next/document'

export default class MyHead extends Head {
  getChunkPreloadLink (filename) {
    // remove as='script' in return statement
  }

  getPreloadDynamicChunks () {
    // remove as='script' in return statement
  }

  render () {
    // in return statement remove as='script' from <link rel='preload'... lines
  }
}

// you should redeclare this function because is not exported
function getPagePathname (pathname, nextExport) {
  if (!nextExport) return pathname
  if (pathname === '/') return '/index.js'
  return `${pathname}/index.js`
}

getPagePathname function has to be declared again because is used by the render method.

Finally, you should change _document.js and import the extended class MyHead:

// _document.js
import Head from './_head'

The AMP validation is successful but you get a warning:

<link rel=preload> must have a valid `as` value`

Here is a discussion about this warning from the AMP Project repo.

image

This is the solution that I've found, probably it's not the best, and it should be reviewed 😉.

Related: #2310

@phongtattuan
Copy link
Author

phongtattuan commented Oct 25, 2017

Thanks so much @ismamz
That's work

Another problem I am facing is using styled-jsx in AMP.

styled-jsx will render many tag <style id='__jsx-xxxx'>..</style> which is not allow on AMP.
AMP only allow 1 tag <style amp-custom>...</style> on the page

can we combine all css to <style amp-custom> with styled-jsx ?

@ismamz
Copy link
Contributor

ismamz commented Oct 25, 2017

You can add the amp-custom attribute to <style> tags generated by styled-jsx, and you can even combine all the CSS code. Look at your _head.js file, the render() method takes the styles array and you can manipulate them.

However, there are some problems: 1) styled-jsx will add class names to custom amp tags and it's not valid, 2) you are intervening the way of work of styled-jsx.

So, maybe styled-jsx is not the best option for an AMP site in this case.

@timneutkens
Copy link
Member

I'm going to close this as we're going to make some changes to better support AMP soon.

@wallismith
Copy link

@timneutkens thanks for the update. Looking forward to AMP changes soon. We have AMP templates and trying to migrate those to Next soon.

@robertoaikode
Copy link

@timneutkens Great! we hope to receive news soon about an excellent integration of AMP with NextJs 😃

@durk0
Copy link

durk0 commented Oct 10, 2018

Any news on AMP support here?

@varungargofb
Copy link

@ismamz I'm currently working on AMP with ReactJs, is there a way to have custom attributes in square brackets such as [class] with reactJs on client side as well. As of now, reactJs when re-render all the custom attributes gets eliminated.
Also tried using react-amphtml, but same happened with AmpHelpers.Bind

@ismamz
Copy link
Contributor

ismamz commented Dec 20, 2018

@varungargofb Hi!, I think that this is not a Next.js related question.
But, the only way I found to use that kind of attributes is setting inner HTML.
For example, if you want to reproduce the example from the amp-bind docs:

<div dangerouslySetInnerHTML={{__html: '<p [text]="\'Hello \' + foo">Hello World</p>'}}/> 
<button on="tap:AMP.setState({foo: 'amp-bind'})">Say "Hello amp-bind"</button>

Other ways like using React.createElement('p', { '[text]': "'Hello ' + foo" }, 'Hello World'); or el.setAttribute('[text]', "'Hello ' + foo"); doesn't work.

I hope that helps you.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 20, 2019
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

7 participants