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

Move amp support to a plugin #2603

Closed
benmccann opened this issue Oct 14, 2021 · 6 comments · Fixed by #4710
Closed

Move amp support to a plugin #2603

benmccann opened this issue Oct 14, 2021 · 6 comments · Fixed by #4710
Labels
Milestone

Comments

@benmccann
Copy link
Member

benmccann commented Oct 14, 2021

Describe the problem

I don't like amp. It's bad for the web

Also, it slows things down a lot:

timeout: 15000 // AMP validator needs a long time to get moving

Describe the proposed solution

Can we use hooks, vite plugins (https://www.npmjs.com/package/vite-plugin-amp), or do we need to add new extension points?

Also, we can potentially move some of the stuff to other options like the proposed inlineCss option (#2620), so that it's not AMP-specific.

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@benmccann benmccann added this to the 1.0 milestone Oct 14, 2021
@JordyMoos
Copy link

Can you please elaborate why it is bad for the web?

I personally think it would be great if svelte has the amp support build in, as it is something used in front end websites.

@Conduitry
Copy link
Member

The short answer is that it's something invented by Google with the primary goal of helping Google gain even more control over more of the internet than they already have.

@JordyMoos
Copy link

Ahh yea i just found out:
https://www.lafoo.com/the-end-of-amp/

Thanks good to know!

@benmccann
Copy link
Member Author

There are many articles about it available online if you'd like to read more: https://www.google.com/search?q=amp+bad+for+web

@Rich-Harris
Copy link
Member

I would love to get rid of built-in AMP support. AMP is flaming hot garbage.

SvelteKit's AMP support consists of the following elements:

  1. Styles are inlined, regardless of size
  2. HTML is run against the AMP validator during dev
  3. import { amp } from '$app/env'
  4. The AMP boilerplate is added
  5. Instead of including all imported styles, it only includes rendered styles, since there's no possibility of components being dynamically added later

Most of this could be done in userland today, especially if we do #3511:

  1. Set inlineStyleThreshold to Infinity
  2. Do validation in handle
  3. VITE_AMP=true npm run dev and import.meta.env.VITE_AMP
  4. Boilerplate can be injected right before validation occurs

The last one is the only part that would need changes to Kit itself, I think. Proposal: if a page is rendered with router: false and hydrate: false — i.e. Kit doesn't include any JS — then we could use rendered CSS (instead of imported CSS) in all cases, rather than making it an AMP-specific feature.

We could then move the validation/boilerplate stuff to a separate package (@sveltejs/amp-kit?) and document the required config changes.

@Rich-Harris
Copy link
Member

Another wrinkle we'll need to accommodate: #4073

@benmccann benmccann added the feature / enhancement New feature or request label Mar 17, 2022
Rich-Harris added a commit that referenced this issue Apr 25, 2022
Rich-Harris added a commit that referenced this issue May 18, 2022
Rich-Harris added a commit that referenced this issue May 19, 2022
Rich-Harris added a commit that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants