Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

Remove global.css from public and move it into app source code #235

Open
brgrz opened this issue Apr 15, 2021 · 6 comments
Open

Remove global.css from public and move it into app source code #235

brgrz opened this issue Apr 15, 2021 · 6 comments

Comments

@brgrz
Copy link

brgrz commented Apr 15, 2021

One of the bigger issues imo for new users coming to Svelte is configuring global styles. Some of that is documented like here
sveltejs/sapper#377 and here https://svelte.dev/docs#style and the Svelte Preprocess is mostly set up nicely to aid in that.

But, the existance of global.css and the reference to it in index.html have always bothered me somewhat since that file is always left out from being build as part of the app.

For my apps, I removed it completely because I wanted the contents to be placed side by side with the app and be part of the build process resulting in being part of the bundled CSS (meaning eventual versioning. cache busting etc.)

I suggest this to be done in the template too, global.css removed and brought into source like so

App.svelte

<style lang="scss">
	@import 'App';	// App component styles (or inlined)
	:global {
		@import 'Media';	// media queries
		@import 'Global';	// global styles
	}
</style>

This would make for a better setup from the get-go. Obv it's a simple change for which I am gladly to make a PR.

@brgrz brgrz changed the title Remove global.css from public and move it into source app code Remove global.css from public and move it into source app code Apr 15, 2021
@brgrz brgrz changed the title Remove global.css from public and move it into source app code Remove global.css from public and move it into app source code Apr 15, 2021
@antony
Copy link
Member

antony commented Apr 15, 2021

I'm not sure if you're suggesting that we should set up preprocessing in the bootstrap application, but if so, that's not something we want to do. It's not a widely held belief by the maintainers that css preprocessing is a good thing.

@brgrz
Copy link
Author

brgrz commented Apr 15, 2021

I can tell you what's for sure not a good thing: having one standalone file defining base styles not being part of the build.

This would work too and it only uses svelte-preprocess without additional plugins in place:

<style>
	:global {
		/* inlined styles copied from public/global.css */
		/* ... */
	}
</style>

@antony
Copy link
Member

antony commented Apr 15, 2021

I can tell you what's for sure not a good thing: having one standalone file defining base styles not being part of the build.

But it's just for base styles, which apply to the base HTML part of the app. All other styles should ideally live inside components inside a style tag.

We don't want to include svelte-preprocess in the default template, sorry. It's something we produce, officially, for people who need a styles preprocessor. However it's not something we'd look to encourage, nor is it something that we think all users require, by default.

Thanks for the suggestion + offer of a PR however!

(by the way, we plan to deprecate this template soon in favour of Svelte KIt - which has a one-key preprocessor installation available out of the box)

@antony antony closed this as completed Apr 15, 2021
@brgrz
Copy link
Author

brgrz commented Apr 15, 2021

I can tell you what's for sure not a good thing: having one standalone file defining base styles not being part of the build.

But it's just for base styles, which apply to the base HTML part of the app. All other styles should ideally live inside components inside a style tag.

Some pretty important parts of styling typically go in there like body margins/paddings, fonts, link styles etc. As it stands now, when that changes it doesn't get published as part of the ongoing build process. I wonder who works that way.

We don't want to include svelte-preprocess in the default template, sorry. It's something we produce, officially, for people who need a styles preprocessor. However it's not something we'd look to encourage, nor is it something that we think all users require, by default.

I really don't understand this, it's included already.

@antony
Copy link
Member

antony commented Apr 15, 2021

I really don't understand this, it's included already.

You're absolutely right. I hadn't noticed it :)

Some pretty important parts of styling typically go in there like body margins/paddings, fonts, link styles etc. As it stands now, when that changes it doesn't get published as part of the ongoing build process. I wonder who works that way.

In this case I'm re-opening as I don't know what the ideal situation is here. I'm fairly sure that we don't want to have a big global wrapping all of our base styles (or, the better solution of a global modifier on the style tag). I'll wait for somebody with a stronger opinion than me to weigh in :)

@antony antony reopened this Apr 15, 2021
@brgrz
Copy link
Author

brgrz commented Apr 15, 2021

In this case I'm re-opening as I don't know what the ideal situation is here. I'm fairly sure that we don't want to have a big global wrapping all of our base styles (or, the better solution of a global modifier on the style tag). I'll wait for somebody with a stronger opinion than me to weigh in :)

I agree, global wrapping looks ugly but this works too

<style global>
	body {
		background-color: blueviolet;
	}
</style>

The problem with this syntax is that I see yellow squiggles under style rules saying "Unused CSS selector" which with wrapping syntax I do not see.

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

No branches or pull requests

2 participants