Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add AmpBoilerplateErrorHandler transformer #1195

Conversation

schlessera
Copy link
Collaborator

@schlessera schlessera commented Apr 9, 2021

This PR adds a transformer that adds a <script amp-onerror> element to the document head to disable the boilerplate early on errors while loading the AMP runtime.

Fixes #1146

@schlessera
Copy link
Collaborator Author

@sebastianbenz
There is a remaining issue because of the whitespace that is added automatically by the JS DOM when adding the script content. This causes the resulting document to fail validation, as the validation rule for this <script amp-onerror> element is extremely strict.

I don't know enough about the JS DOM abstraction to solve that. Any ideas on how to ensure no whitespace is added?

@sebastianbenz
Copy link
Collaborator

sebastianbenz commented Apr 9, 2021

The transformer is fine, we run a code formatter after the transformations (for readability) which adds the whitespace. You have to disable this, by adding script here:

'unformatted': ['noscript', 'style'],

You'll also have to run npm run test:snapshot:optimizer again.

@schlessera schlessera force-pushed the add/1146-amp-boilerplate-error-handler branch from 4054038 to f632431 Compare April 9, 2021 20:10
@schlessera
Copy link
Collaborator Author

The CLA check seems to be stuck here...

@schlessera
Copy link
Collaborator Author

When disabling the beautify on scripts, it not only leaves the internals of each script tag intact, but it also removes the newlines between the individual elements. Is there a way to avoid styling the script internals, but keep the newlines outside of the script element intact?

@sebastianbenz
Copy link
Collaborator

Not sure if that's configurable. I don't consider this a critical problem.

@sebastianbenz sebastianbenz merged commit be2092a into ampproject:main Apr 12, 2021
@schlessera schlessera deleted the add/1146-amp-boilerplate-error-handler branch April 13, 2021 17:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for script amp-onerror
2 participants