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

Add @snowpack/plugin-postcss-transform #684

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

drwpow
Copy link
Collaborator

@drwpow drwpow commented Jul 31, 2020

Changes

Adds the ability to transform all .css files via PostCSS. This plugin can process source .css files, as well as CSS generated dynamically from Vue and Svelte files.

Pros
Because it’s a transform() plugin, it can run after everything. Vue, Svelte, plain CSS… you name it, PostCSS will transform it. This is really handy and pretty universal thanks to how Snowpack is structured. It really leans into Snowpack’s strengths and embodies the “unbundled” philosophy.

Cons
Because it doesn’t run in load(), it can’t remap files. So PostCSS can’t take advantage of postcss-import or other plugins where your CSS is completely combined & concatenated. However, if you really need to concatenate/bundle your CSS, you’re probably not just combining files as that may have unintended consequences. You’re probably thinking about that earlier, and either using CSS modules or Sass.

I am leaning toward this plugin adding value to the Snowpack ecosystem overall. Even though it can’t bundle, it can touch all CSS generated by any other plugin, which seems super valuable in and of itself.

Testing

When added to the local app-template-vue template, it was able to successfully minify the CSS and add browser prefixes (browser prefixes not necessarily shown, but the minification working here means it was also prefixing, too):

Screen Shot 2020-07-31 at 12 06 27 AM

@drwpow drwpow requested a review from a team as a code owner July 31, 2020 06:11
@vercel
Copy link

vercel bot commented Jul 31, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/d24pdsfiv
✅ Preview: https://snowpack-git-drwpow-plugin-postcss-transform.pikapkg.vercel.app

const flags = [];
if (config) flags.push(`--config ${config}`);

const {stdout} = await execa('postcss', flags, {
Copy link
Collaborator Author

@drwpow drwpow Jul 31, 2020

Choose a reason for hiding this comment

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

We want to use the PostCSS CLI because a) it becomes a dependency that the user version-manages, not Snowpack (we don’t want to lock users to an old version), and b) it lets PostCSS be more intelligent with how it loads its own configuration and plugins (also c) CLI tools tend to be faster too).

input: contents,
});

if (stdout) return stdout;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Plugin thought: in transform(), we only allow strings. However, PostCSS can also generate source maps (.css.map). Do we want to allow transform to return an object, too, to generate new files?

Copy link
Owner

Choose a reason for hiding this comment

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

working through this in #681 !

Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

LGTM!

I'd want you to give one more thought to the load() vs. transform() decision, only because the reasons that you gave to run PostCSS on all generated CSS (instead of source CSS) seemed more like optimize use-cases than load/build ones. Can you think of a real use-case for using something else to use build/load to generate CSS, and then wanting to run PostCSS on that output?

@drwpow
Copy link
Collaborator Author

drwpow commented Aug 4, 2020

Can you think of a real use-case for using something else to use build/load to generate CSS, and then wanting to run PostCSS on that output?

Yeah, good question. I think the Vue community is the major thinking here—they’d want PostCSS to be run in build and dev mode after the compiler (after load()), but for dev that’d need to happen before optimize(), so transform() feels like the right option.

I think CSS bundling would be the only thing better suited for optimize, however, I think that use case is rare. If you need to bundle CSS, order is a huge concern (same as JS). So you’re probably using a more sophisticated tool, such as Sass, to begin with. Which means you’re thinking about it earlier in the process.

I don’t think this implementation is an absolutely perfect approach that will work for everyone (assuming that even exists), but I think this living in transform() will provide the most value for most devs.

} else if (result && typeof result === 'object' && (result as {result: string}).result) {
output[destExt] = {code: (result as {result: string}).result, map};
output[destExt].code = (result as {result: string}).result;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After just merging the source mapping PR, I wanted to do a little cleanup here. This doesn’t change execution, but it’s a little safer. I’ve found that source maps were easy to drop in the transformation step here.

@drwpow drwpow merged commit 08c7f04 into master Aug 5, 2020
@drwpow drwpow deleted the drwpow/plugin-postcss-transform branch August 5, 2020 15:11
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.

None yet

2 participants