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

Styles disappear in embroider production build when there are css layers in app.css #1358

Open
candunaj opened this issue Feb 14, 2023 · 8 comments

Comments

@candunaj
Copy link

candunaj commented Feb 14, 2023

When you specify CSS layers at the top of app.css then imports below it disappear in the embroider production build. The development build works as expected (nothing is removed from app.css). Styles from imported file are used in template, so they couldn't be removed as a part of any optimization process. I have tried it in the ember app without embroider and it works correctly.

So when you specify the following app.css file

/* app.css */
@layer base, components;
@import 'global.css' layer(base);

h1{
  color: red;
}
/* global.css */
h2{
  color: green;
}

when you run ember serve --environment=production then you get the following app.css in the dist/assets directory:

/* app.css */
@layer base, components;
h1{
  color: red;
}

Import disappeared!

When you move defining layers under the imports then everything works as expected and nothing is removed from app.css

/* app.css */
@import 'global.css' layer(base);
@layer base, components;

h1{
  color: red;
}

I have put together a repo with the issue here.

@NullVoxPopuli
Copy link
Collaborator

Are you using postcss/tailwind? If so, imports are only supported at the top of the file (non embroider issue).

See: https://tailwindcss.com/docs/using-with-preprocessors#using-post-css-as-your-preprocessor

@mansona
Copy link
Member

mansona commented Feb 14, 2023

@NullVoxPopuli the reproduction is pretty vanilla so no it's not using any of those things

@NullVoxPopuli
Copy link
Collaborator

I've just never seen layer and stuff without additional config / tools 🤷

@ef4
Copy link
Contributor

ef4 commented Feb 14, 2023

Discussed this at embroider office hours, the next action suggestion is to try reproducing this in a plain webpack project first to rule that out. You can find the style configuration we're using here:

private setupStyleConfig(variant: Variant): {

{
test: isCSS,
use: styleLoaders,
},

@candunaj
Copy link
Author

Thank you, I will try to reproduce it in a plain webpack project.

@candunaj
Copy link
Author

candunaj commented Feb 15, 2023

1. setupStyleConfig is used only for imported CSS into a javascript file

I have found, that stylePlugins and styleLoaders are used only for CSS files imported in a javascript file. In our case, app.css is not imported to any javascript file, so stylePlugins and styleLoaders don't have any effect on app.css.
I have confirmed that by commenting stylePlugins and styleLoaders out from ember-webpack. The app.css file was still in dist/assets/ directory.

2. plugin for copying /styles/*.css to dist/assets/

I have found that our styles appear in assets folder in vanilla app and then they are copied into dist/assets by the following plugin:

compiler => {
compiler.hooks.done.tapPromise('EmbroiderPlugin', async stats => {
this.summarizeStats(stats, variant, variantIndex);
await this.writeFiles(this.bundleSummary, appInfo, variantIndex);
});
},

That plugin does multiple things, but it also minify app.css and then write it

const minifiedCss = csso.minify(cssContent).css;
let finalFilename = this.getFingerprintedFilename(style, minifiedCss);
outputFileSync(join(this.outputPath, finalFilename), minifiedCss);

I have debugged it, and my import disappear in minifying process

const minifiedCss = csso.minify(cssContent).css;

I will continue investigating csso.minify further.

3. Question

I am just curious, why we don't import app.css to a javascript file and then stylePlugins and styleLoaders would deal with styles? I have created a plain webpack project as you suggested with the same styleLoaders and stylePlugins as we have in embroider and when I imported styles in the main javascript file it was working as expected.
Here is a repo with simple webpack and layers working correctly with plugins and loaders and styles imported in main.js file.
https://github.com/candunaj/webpack-layers

@ef4
Copy link
Contributor

ef4 commented Feb 15, 2023

I am just curious, why we don't import app.css to a javascript file and then stylePlugins and styleLoaders would deal with styles?

This is because we're trying to be compatible with how app.css gets handled in a classic build, and that handling is unfortunately very implementation-defined and dynamic. It's hard to change it without breaking apps.

So to embroider, app/styles/app.css (and all the rules about how addons influence it) is a legacy feature. We could allow people to opt out of it working the way it does, but the default is the backward-compatible thing.

I will continue investigating csso.minify further.

Great, this seems like the likely cause of the bug. I don't have any particular opinion about what minimizer runs there. Probably it should be the same one that style-loader (or MiniCSSExtractPlugin, which is the production default) is using on the webpack-handled styles.

We have a similar situation with vendor.js: it's a legacy feature that doesn't go through webpack because webpack doesn't have the greatest support for script (as opposed to module) context. So embroider directly handles minification of that file, and I believe it tries to use the same minified that webpack is using on the webpack-handled JS.

@kategengler
Copy link

I am hitting a variant of this where csso removes @media tags, documented as an issue on csso css/csso#464

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

No branches or pull requests

5 participants