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

🐛 BUG: PostCSS example from documentation does not work (and some plugins are broken) #3528

Closed
1 task
felixsanz opened this issue Jun 6, 2022 · 28 comments · Fixed by #3685
Closed
1 task
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@felixsanz
Copy link
Contributor

felixsanz commented Jun 6, 2022

What version of astro are you using?

v1.0.0-beta.40

Are you using an SSR adapter? If so, which one?

no

What package manager are you using?

npm

What operating system are you using?

Linux

Describe the Bug

https://docs.astro.build/en/guides/styling/#postcss

Documentation says that PostCSS integration can be enabled with just creating a postcss.config.js file, install some plugins and require them there.

And it works with plugins like autoprefixer, but postcss-nesting does not work. The example in the documentation uses postcss-preset-env (which also includes postcss-nesting).

In the test case i created i just forked the "Just the basics" repo example, installed the plugin, added postcss.config.js and moved the css inside components/Card.astro so it becomes:

	.link-card {
		list-style: none;
		display: flex;
		padding: 0.15rem;
		background-image: var(--link-gradient);
		background-size: 400%;
		border-radius: 0.5rem;
		background-position: 100%;
		transition: background-position 0.6s cubic-bezier(0.22, 1, 0.36, 1);

		& > a {
			width: 100%;
			text-decoration: none;
			line-height: 1.4;
			padding: 1em 1.3em;
			border-radius: 0.35rem;
			color: var(--text-color);
			background-color: white;
			opacity: 0.8;
		}

1654474144

The nesting thing does not work, devtools shows an error because invalid property. In the case of doing this in a svelte component... the dev server crash and doesn't let you continue.

Looks like postcss plugins that include (at the moment) a custom syntax does not work

Link to Minimal Reproducible Example

https://codesandbox.io/s/silent-dew-fehxqz

Participation

  • I am willing to submit a pull request for this issue.
@natemoo-re natemoo-re added - P4: important Violate documented behavior or significantly impacts performance (priority) s2-medium labels Jun 6, 2022
@oliverturner
Copy link

oliverturner commented Jun 8, 2022

As a Stage 1 proposal, nesting must be opted into explicitly (postcss-nesting only enables Stage 2 and later by default).

So not a bug per se, but since nesting is a common-enough want, an update to the docs would probably be helpful. PR incoming!

https://codesandbox.io/s/fancy-surf-h0i1wk?file=/postcss.config.js

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 8, 2022

As a Stage 1 proposal, nesting must be opted into explicitly (postcss-nesting only enables Stage 2 and later by default).

Nesting is a common-enough want that it may warrant an update to the docs?

https://codesandbox.io/s/fancy-surf-h0i1wk?file=/postcss.config.js

Ok about that, but postcss-nesting does not work. And in svelte components it crash and don't even let you start the server, so it's a valid bug

@oliverturner
Copy link

Is the rendered example in that fork not working for you?

https://codesandbox.io/s/fancy-surf-h0i1wk?file=/src/components/Card.astro

Screenshot 2022-06-08 at 16 25 05

@felixsanz
Copy link
Contributor Author

nope...

1654705512

@oliverturner
Copy link

oliverturner commented Jun 8, 2022

Ah... that's because I was looking into the Svelte set-up: should have forked before doing that!

Try this: https://codesandbox.io/s/lively-sun-9p6qyo?file=/src/components/Card.astro

@felixsanz
Copy link
Contributor Author

Ah... that's because I was looking into the Svelte set-up: should have forked before doing that!

Try this: https://codesandbox.io/s/lively-sun-9p6qyo?file=/src/components/Card.astro

That doesn't include a .svelte component :/

@oliverturner
Copy link

Yes, I was trying to validate that the PostCSS integration works as described with Astro components.

@oliverturner
Copy link

So this version uses CSS nesting in Svelte modules

https://codesandbox.io/s/admiring-raman-ftigzk?file=/src/components/Card.svelte

@felixsanz
Copy link
Contributor Author

So this version uses CSS nesting in Svelte modules

https://codesandbox.io/s/admiring-raman-ftigzk?file=/src/components/Card.svelte

can you check a local project using vscode? to me crash both language-tools and the dev server

@oliverturner
Copy link

oliverturner commented Jun 9, 2022

Can you try updating astro.config.mjs with the following and let me know how you get on?

import { defineConfig } from "astro/config";
import svelte from "@astrojs/svelte";
import preprocess from "svelte-preprocess";

export default defineConfig({
  integrations: [
    svelte({
      preprocess: [
        preprocess({
          postcss: true,
        }),
      ],
    }),
  ],
});

@felixsanz
Copy link
Contributor Author

Can you try updating astro.config.mjs with the following and let me know how you get on?

import { defineConfig } from "astro/config";
import svelte from "@astrojs/svelte";
import preprocess from "svelte-preprocess";

export default defineConfig({
  integrations: [
    svelte({
      preprocess: [
        preprocess({
          postcss: true,
        }),
      ],
    }),
  ],
});

Yeah i saw you importing the svelte-preprocess package in some example and i tried that already, it didn't help :(

@oliverturner
Copy link

Can you confirm that this repo doesn't work for you? I'm running it locally without issue.
https://github.com/oliverturner/astro-svelte-postcss

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 9, 2022

Can you confirm that this repo doesn't work for you? I'm running it locally without issue. https://github.com/oliverturner/astro-svelte-postcss

i don't understand the magic going on! your repo works perfect. mine... after updating from b40 to b42 it doesn't crash anymore, but i can't get rid of the error that vscode shows!

1654795284

i reloaded astro language server, svelte language server, reloaded editor window, closed and opened a new editor 😂

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 9, 2022

Could this be the issue?

Importing postcss-nesting and hovering it i get this message:

Could not find a declaration file for module 'postcss-nesting'. '.../node_modules/postcss-nesting/dist/index.cjs' implicitly has an 'any' type.
  Try `npm i --save-dev @types/postcss-nesting` if it exists or add a new declaration (.d.ts) file containing `declare module 'postcss-nesting';`ts(7016)

EDIT: probably not


I remember in sveltekit i had to add this to my vscode settings.json:

  "svelte.plugin.css.diagnostics.enable": false,

but it's not working here, no effect

@oliverturner
Copy link

Just to refocus on the Astro-specific aspects, here's what I think one can say

  • PostCSS: The documentation could be usefully enhanced with an example of how to opt in to plugin-specific options such as postcss-preset-env's.

    As I said above, I'm happy to make a PR for this.

  • Svelte: it feels like @astrojs/svelte should automatically add the necessary preprocess config... and I've got a feeling that it (or its precursor) used to?

    More research required!

@oliverturner
Copy link

oliverturner commented Jun 9, 2022

Re the Svelte Language Server complaining about CSS syntax it doesn't recognise, that feels like a challenge that needs ought to be to be addressed upstream in the vscode-css-languageservice: I think users should be able to opt in to early stage features (such as nesting at Stage 1, or Custom Environment Variables at Stage 0) without getting chastised!

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 9, 2022

Re the Svelte Language Server complaining about CSS syntax it doesn't recognise, that feels like a challenge that needs to be addressed upstream in the vscode-css-languageservice: I think users should be able to opt in to early stage features (such as nesting at Stage 1, or Custom Environment Variables at Stage 0) without getting chastised!

but in sveltekit this works fine! maybe astro language-tools are not reading that "svelte.plugin.css.diagnostics.enable": false thing. or it's not even implemented. this is part of svete-preprocess if i'm right.

@Princesseuh you are in charge of language-tools right? Can you shed some light? (if you know)

@Princesseuh
Copy link
Member

Princesseuh commented Jun 9, 2022

Our language tools are only running in Astro files, errors inside Svelte files are handled by the Svelte extension only.

We can't show errors inside Svelte files, they can't show errors inside Astro files

(Our language server does not even support CSS validation, so, we can't even show errors inside style tags in general)

You shouldn't actually need that setting as the Svelte language server disable validation for style tags with lang set to PostCSS (since yeah, vscode-css-languageservice does not support PostCSS)

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 10, 2022

Then whose fault is it? 😆

All i can say is..

  1. I had a custom/own TS project with no frameworks and this worked fine, so not vscode's fault.
  2. This also works fine with sveltekit, so svelte seems to be compatible and doesn't complain either.
  3. What's left? Astro?

@Princesseuh I think it's needed! Comment from svelte's language tools author: sveltejs/language-tools#190 (comment) (and I can confirm that you NEED it on sveltekit or the error won't dissapear)

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 10, 2022

@oliverturner @Princesseuh I found the issue. Astro still needs a svelte.config.js file for this to work:

1654863221

1654863191

So Astro is missing something?

astro.config.js (does NOT fix it):

import { defineConfig } from "astro/config";
import svelte from "@astrojs/svelte";
import sveltePreprocess from "svelte-preprocess";

import nesting from 'postcss-nesting'

const postcssConfig = {
	plugins: [
		nesting,
	],
}

export default defineConfig({
  integrations: [
    svelte({
      preprocess: [
        sveltePreprocess({
          postcss: postcssConfig,
        }),
      ],
    }),
  ],
});

svelte.config.js (DOES fix it):

const sveltePreprocess = require('svelte-preprocess')
const nesting = require('postcss-nesting')

const postcssConfig = {
	plugins: [
		nesting,
	],
}

const config = {
	preprocess: [
		sveltePreprocess({
			postcss: postcssConfig,
		}),
	],
}

module.exports = config

@Princesseuh
Copy link
Member

Princesseuh commented Jun 10, 2022

Astro pass those settings directly to Svelte, whereas the Svelte language server expect an actual Svelte config file, I don't think there's anything we can do apart from documenting that needs

(Unless the Svelte team is interested in a PR to their language tool to load our config)

Thank you for finding the issue!

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 10, 2022

Ah, yeah the Svelte language server load that config. Not much we can do on our side apart from documenting that need

Thank you for finding the issue!

So having this config inside astro.config.js is useless and does nothing?

  integrations: [
    svelte({
      preprocess: [
        preprocess({
          postcss: postcssConfig,
        }),
      ],
    }),

or Astro still uses it to preprocess stuff in svelte components throught vite? (a.k.a.: do i need the same code twice? 😆😆)

@Princesseuh
Copy link
Member

Ah, yeah the Svelte language server load that config. Not much we can do on our side apart from documenting that need

Thank you for finding the issue!

So having this config inside astro.config.js is useless and does nothing?

  integrations: [
    svelte({
      preprocess: [
        preprocess({
          postcss: postcssConfig,
        }),
      ],
    }),

or Astro still uses it to preprocess stuff in svelte components throught vite? (a.k.a.: do i need the same code twice? 😆😆)

Yep, that config is needed for the Svelte integration to process the files, maybe we could update the integration to load a Svelte config file

@felixsanz
Copy link
Contributor Author

felixsanz commented Jun 10, 2022

Since svelte() accepts what svelte.config.js outputs... documentation could explain it like this:

import svelteConfig from 'svelte.config.js'

export default defineConfig({
  integrations: [
    svelte(svelteConfig),
  ],
});

Also a change to make it work automatic sounds nice. I think if Astro still depends on other framework's language tools for this kind of things, per-framework specific config file are still needed.

Thank you both for helping me debug the issue!

@bholmesdev
Copy link
Contributor

  • Svelte: it feels like @astrojs/svelte should automatically add the necessary preprocess config... and I've got a feeling that it (or its precursor) used to?

@oliverturner Ah yep, it did used to autoconfigure postcss! We had to remove this after some gnarly console errors for non-postCSS users, complaining "PostCSS configuration was not passed or is invalid." See #2273

Interesting that leaving out postcss: true lets in some PostCSS rules and not others though 😕 That explains why your test suite didn't catch it. Might be worth a) documenting or b) smartly configuring postcss to true if a postcss config is present in your project!

@bholmesdev
Copy link
Contributor

Also @oliverturner I'd love to see some beefed-up PostCSS docs there! I'll defer to your expertise but lmk if I should jump in as well.

@bholmesdev bholmesdev added - P2: has workaround Bug, but has workaround (priority) and removed - P4: important Violate documented behavior or significantly impacts performance (priority) labels Jun 21, 2022
@bholmesdev
Copy link
Contributor

@felixsanz Found a fix for your Svelte issue! #3685 I noticed we weren't applying our generated PostCSS config to Svelte. This should apply all Post CSS config by default, including Tailwind defaults if you mix with that integration 👍

@JonathonRP
Copy link

@bholmesdev how can I achieve this with latest version of astrojs/svelte?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants