Skip to content

Use rendered rather than emitted CSS in AMP mode #808

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

Closed
Rich-Harris opened this issue Mar 31, 2021 · 4 comments · Fixed by #1408
Closed

Use rendered rather than emitted CSS in AMP mode #808

Rich-Harris opened this issue Mar 31, 2021 · 4 comments · Fixed by #1408
Labels
feature / enhancement New feature or request

Comments

@Rich-Harris
Copy link
Member

Is your feature request related to a problem? Please describe.
AMP pages have a strict (and somewhat arbitrary) limit on the amount of CSS a page can include. They also provide a guarantee that doesn't exist in non-AMP mode, namely that if condition is false at SSR time...

<script>
  import Foo from '$lib/components/Foo.svelte';

  export let condition;
</script>

{#if condition}
  <Foo/>
{/if}

...we can safely omit the styles emitted by Foo.svelte.

It would be nice if we could take advantage of that and only include styles for components that were rendered, to increase the likelihood of staying beneath the 75kb ceiling.

Describe the solution you'd like

  • Configure the Svelte plugin to not emit CSS in AMP mode
  • Use rendered.css instead of inspecting the module graph

How important is this feature to you?
It depends on the outcome of the research we plan to conduct on how much traffic our app would lose if we told AMP to bugger off once and for all.

@Rich-Harris Rich-Harris added the feature / enhancement New feature or request label Mar 31, 2021
@quantuminformation
Copy link

I'm not too bothered by it, I just had to Google about it today when I noticed amp in the code

image

@jerriclynsjohn
Copy link

@Rich-Harris I think its absolutely relevant to also think about adding an option for AMP to output AMP compliant HTML to a different folder, apart from the actual HTML. I've given an example in this issue #1308. Also, how can we handle CSS when we use preprocessor, I've described the problem that I faced when I'm using tailwindCSS in this issue #1310

@benmccann
Copy link
Member

Is there any reason to use AMP anymore? Google no longer prioritizes it in search results: https://www.lafoo.com/the-end-of-amp/. I would think you'd be better off not using it and that we'd be better off to drop support for it

@jerriclynsjohn
Copy link

I hope they ditch AMP too but they've recently invested in AMP stories and they are promoting that pretty strongly. I think Google will improve AMP implementation instead of just dropping it.

Rich-Harris added a commit that referenced this issue May 10, 2021
Rich-Harris pushed a commit that referenced this issue May 11, 2021
* failing test for #808

* use rendered CSS for AMP pages, rather than emitted CSS - fixes #808

* bump vite-plugin-svelte

* oops
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants