egoist / tsup

The simplest and fastest way to bundle your TypeScript libraries.
https://tsup.egoist.dev
MIT License
8.95k stars 217 forks source link

postcss import resolution not working when css loader set to "text" (monorepo only) #770

Closed rondonjon closed 1 year ago

rondonjon commented 1 year ago

743 has requested support CSS inlining as an alternative to generating additional output files.

The suggested solution was to set the .css loader to text, which wasn't working initially but has been fixed in tsup 6.3.0.

While the contents are now "inlined", it seems that they are no longer preprocessed by postcss, as one would expect from "CSS inlining".

With the default settings, tsup generates an external CSS of more than 150 KB in size ...

IIFE ../web/public/app/index.css 150.75 KB

... but after setting the .css loader to text, I am now getting a very short (inlined) CSS snippet which still contains imports that postcss should have resolved:

@import './themes/a.css';
@import './themes/b.css';
@import './themes/c.css';

Actually I find this behavior quite logical and consistent and would propose to leave it just how it is. If the content were preprocessed by postcss, the term (plain) "text" would be misleading. But that obviously doesn't solve the original request.

Could we have a new loader option inline-css to make this more explicit and meet the original request?

Or, more generically, would it make sense to split these configs into at least two parameters, "loading" (inlined vs. extra file) and "transformation" (e.g. typescript, svelte, css)?

await-ovo commented 1 year ago

If you want to resolve paths for @import rules when switching .css loader to text, I think just adding postcss-import plugin to your postcss.config.jswill solve your problem.

rondonjon commented 1 year ago

There is already a postcss.config.js with the following contents:

plugins: [
    'postcss-import',
    'tailwindcss/nesting',
    'autoprefixer',
    'tailwindcss',
  ],

This config is working fine when with the "css" loader and will then produce a big standalone CSS file which has all CSS imports resolved. With the "text" loader however the config seems to be ignored.

await-ovo commented 1 year ago

Hi @rondonjon, I am interested in this problem, could you please provide a repository to reproduce this?

image

In my case, it seems that postcss works fine with the text loader, here is dist/index.js for above example:

image
rondonjon commented 1 year ago

Thanks @await-ovo!

Here you go: https://gitlab.com/rondonjon/tsup-postcss-reproduction

While setting this up I noticed that the problem only occurs in a yarn monorepo. But then it is fully reproducible:

Test base

Style output with tsup and "css" loader

Expected result: a.css and b.css are merged, minified, and written to index.css Actual result: 🟢

body{padding:0;margin:0}html{background:red}

Style output with tsup and "text" loader

Expected result: a.css and b.css are merged. minified, and inlined in index.js Actual result: 🔴 import of b.css is not resolved, CSS is not minified -> is postcss executed at all ???

"use strict";(()=>{var r=`@import "./b.css";

html {
  background: red;
}
`;alert(r);})();
await-ovo commented 1 year ago

Currently tsup search postcss.config.js based on the path the css file, and if it doesn't find it, it skips the postcss transform, because there is no postcss.config.js in packages/styles directory, so @app/styles/a.css will not be processed by postcss.

The temporary solution is add a postcss.config.js to the packages/styles directory so that @import rule can be transformed.

I think tsup should search for postcss.config.js in the root of the project directory like vite does

rondonjon commented 1 year ago

@await-ovo, thanks again, this is really helpful, and working around the issue is good enough for me at the moment. Out of curiosity, could you explain why this only affects the "text" loader but not the "css" loader?

await-ovo commented 1 year ago

@await-ovo, thanks again, this is really helpful, and working around the issue is good enough for me at the moment. Out of curiosity, could you explain why this only affects the "text" loader but not the "css" loader?

i think esbuild itself will resolve @import in css files with css loader~

rondonjon commented 1 year ago

What is your take on this @egoist ?

tsup is resolving the location of the postcss.config.js file differently in the "text" and the "css" loader when the imported css file is outside the tsup directory (here: other package in same monorepo).

It seems arguable which resolution makes more sense (based on the location of the source file or on the tsup/project root), personally I tend towards the tsup/project root, but I think the resolution should be consistent across all loaders.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 6.6.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: