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

[feat] Compiler option css: 'none' support #7914

Merged
merged 7 commits into from Nov 9, 2022

Conversation

maxiruani
Copy link
Contributor

Why?

I found that generating client side hydratable code with css: false or css: true end up processing styles entirely unnecessary.
When hydrating SSR components, the styles are commonly already generated and there is no need to generate it again for the client side bundle.
This will speed up the build development process because it completely avoids processing it.

Details

  • I have also added support for more descriptive 'injected' (formerly true) and 'external' (formerly false) css option values. It still supports boolean values but it raises deprecated warning.
  • Added support for 'none' css option.
  • Added test for css: 'none' option.

References

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@maxiruani
Copy link
Contributor Author

@benmccann Thanks for the advice. It should be fine now. Let me know if you need anything else from me.

@benmccann benmccann linked an issue Oct 6, 2022 that may be closed by this pull request
src/compiler/parse/read/style.ts Outdated Show resolved Hide resolved
src/compiler/compile/index.ts Outdated Show resolved Hide resolved
src/compiler/compile/index.ts Outdated Show resolved Hide resolved
@benmccann
Copy link
Member

@maxiruani it looks like this PR needs a rebase

Copy link
Member

@baseballyama baseballyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. But I think we need to update the document.
https://svelte.dev/docs#compile-time-svelte-compile

@maxiruani
Copy link
Contributor Author

@maxiruani it looks like this PR needs a rebase

Hi @benmccann, sorry for the delay, I've been on vacations. Does the PR still needs a rebase?

@benmccann
Copy link
Member

Yes, this PR still needs to be updated. You can see that it says "This branch has conflicts that must be resolved". And the docs should be updated as @baseballyama suggested

@maxiruani
Copy link
Contributor Author

@benmccann I think it should be fine now. Sorry for the trouble 😄. Let me know if you need anything else from me.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rename compiler css option
4 participants