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

Drop user <script> tags #6688

Closed
Timer opened this issue Mar 17, 2019 · 2 comments · Fixed by #6830
Closed

Drop user <script> tags #6688

Timer opened this issue Mar 17, 2019 · 2 comments · Fixed by #6830
Assignees
@ijjk

Comments

@Timer
Copy link
Member

Timer commented Mar 17, 2019

When the user includes a custom <script> tag on an Amp page, we should remove it before streaming the response to the client.

This should probably happen before we run Amp validation because we can give a better error to the user about what's going on. We can swap these orders in the future.

This will ensure users don't see behaviors which shouldn't be possible.
@Timer Timer added this to the Amp Support milestone Mar 17, 2019
@timneutkens
Copy link
Member

Ideally this would be detected at compile time.

@Timer Timer assigned Timer and ijjk Mar 19, 2019
@BosenY
Copy link

BosenY commented Mar 22, 2019

I think we need a tag to determine if the page is an AMP page, so we can build both AMP and normal pages in one app.

@Timer Timer removed their assignment Mar 25, 2019
@ijjk ijjk mentioned this issue Mar 29, 2019
Merged
timneutkens pushed a commit that referenced this issue Mar 29, 2019
@ijjk @timneutkens
Add dropping of custom scripts in AMP mode (#6830)
26a4eb8 
Drops user `<script>` tags and shows a warning in AMP mode. Right now they are only dropped in production mode and left in dev mode so the validator shows its warning since it looks like conflicting props log messages are being cleared causing them to not show. 

Closes: #6688
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ijjk ijjk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add dropping of custom scripts in AMP mode ijjk/next.js
4 participants
@Timer @timneutkens @BosenY @ijjk