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

Updating postcss-preset-env from v6.7.1 to v7.X causes postcss-css-variables to error #319

Closed
nickyblissAviva opened this issue Mar 25, 2022 · 11 comments

Comments

@nickyblissAviva
Copy link

We are currently using postcss-preset-env 6.7.1 with postcss 8.4.12, postcss-css-variables 0.18.0 and postcss-import 14.1.0 and when we have tried to update out packages to use the latest version of postcss-preset-env out build scripts start to throw errors for variables being undefined and used without a fallback.

Our current css processing pipeline has a core css file that contains only css imports to our various CSS component files. The first imports in the file contain all the css variables used within the other imported css files.

@import url('core/core-fonts.css');
@import url('core/variables-styles-core.css');
@import url('core/variables-icons-core.css');
@import url('core/core.css');
@import url('core/headings.css');
@import url('core/buttons.css');

On moving to any v7 version of postcss-preset-env the variables are no longer being found and replaced within the other imported css files as they were originally.

postcss-css-variables: C:\site\assets-watched\css\core\buttons.css:290:2: variable --focus-plain is undefined and used without a fallback
@Antonio-Laguna
Copy link
Member

Antonio-Laguna commented Mar 25, 2022

Hi @nickyblissAviva could you share more about your config? Whether that's the postcss.config.js or any Webpack config it'd be great to have that around.

@nickyblissAviva
Copy link
Author

This is our postcss processing function that is passed into postcss. Is this what you need?

// Processor functions that are run on the css, runs the imports and provides compatibility; applies custom colour variables if provided
const getProcessors = ( coreCssVars, customCssVars ) => {
	'use strict';

	const twentyTwentyOneCssVarsPath = '../../assets-raw/core/base/base/css/variables-colours-fonts-logos-2021.js';
	delete require.cache[ require.resolve( twentyTwentyOneCssVarsPath ) ]; // Delete cached variables file ready to reload

	const workingCssVarsPath = '../../assets-raw/core/base/base/css/variables-colours-fonts-logos.js';
	delete require.cache[ require.resolve( workingCssVarsPath ) ]; // Delete cached variables file ready to reload

	const vars = {},
		workingCssVars = require( workingCssVarsPath ),
		twentyTwentyOneCssVars = require( twentyTwentyOneCssVarsPath );

	let baseVars;
	if ( coreCssVars === 'twentyTwentyOne' ) {
		baseVars = twentyTwentyOneCssVars.variables;
	} else if ( coreCssVars ) {
		baseVars = coreCssVars.variables;
	} else {
		baseVars = workingCssVars.variables;
	}

	if ( customCssVars ) {
		// Dots are spread operators; each step will take the previous one and overwrite but overwrite any overlaps
		vars.variables = { ...baseVars, ...themingConfig.css.overrides, ...customCssVars };
	} else {
		vars.variables = { ...baseVars };
	}

	return [
		cssImport(), // Reads @import rules
		postcssPresetEnv({ // Determines required css polyfills
			browsers : [ '> .2% and last 2 major versions', 'last 2 ChromeAndroid versions', 'Firefox ESR', 'ie 11', 'Edge >= 17' ],
			stage : 2,
			features : {
				'custom-properties' : { preserve : false },
			},
		}),
		cssVariables( vars ), // Resolves variables to static values
	];
};

@Antonio-Laguna
Copy link
Member

@nickyblissAviva checking this I think you should disable custom-properties. css-variables seems to be a non-standard replacement for custom-properties so I wouldn't mix them. The issue at first glance is that custom-properties is removing the variables given that preserve is false.

Could you try with this?

return [
	cssImport(), // Reads @import rules
	postcssPresetEnv({ // Determines required css polyfills
		browsers : [ '> .2% and last 2 major versions', 'last 2 ChromeAndroid versions', 'Firefox ESR', 'ie 11', 'Edge >= 17' ],
		stage : 2,
		features : {
			'custom-properties' : false,
		},
	}),
	cssVariables( vars ), // Resolves variables to static values
]

@nickyblissAviva
Copy link
Author

Thank you. I wonder if thats is something that has been left in from when we first setup our CSS pipelines a number of years ago.

Good news is I have no errors streaming up my terminal window now, so fingers crossed all is fine. I'll have to do a good chunk of testing to make sure the css coming out is still ok.

@Antonio-Laguna
Copy link
Member

I'm crossing my fingers as well! 🤞

Let us know if you find anything that's not working as expected!

@nickyblissAviva
Copy link
Author

Looks like there are some differences in the css file outputs.

So original output was:

.js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-dark .t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-card-accent-light .m-card .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: url('../images/icons/sprites/arrow-up.svg');
}

now its:

.js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: url('../images/icons/sprites/arrow-up.svg');
}

.t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: url('../images/icons/sprites/arrow-up.svg');
}

.t-accent-dark .t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: url('../images/icons/sprites/arrow-up.svg');
}

.t-card-accent-light .m-card .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: url('../images/icons/sprites/arrow-up.svg');
}

Now in our original css file with the css variables in the css is formatted with all 4 rules together so these are now being split apart when being run through postcss.

Any ideas?

@Antonio-Laguna
Copy link
Member

@nickyblissAviva can you provide the source CSS?

@nickyblissAviva
Copy link
Author

Stripped right back for you but basically how it all hangs together.

variables-icons-core.css

:root {
	--icon-arrow-up: url('../images/icons/sprites/arrow-up.svg');
}

o-faq-panel.css

.js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-dark .t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-card-accent-light .m-card .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
	background-image: var(--icon-arrow-up);
}

help.css

@import url('core/variables-icons-core.css');
@import url('core/o-faq-panel.css');

@Antonio-Laguna
Copy link
Member

@nickyblissAviva unfortunately this is happening due to postcss-css-variables. I've ran this CSS:

:root {
  --icon-arrow-up: url('../images/icons/sprites/arrow-up.svg');
}

.js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-accent-dark .t-accent-light .js-o-faq-panel .is-open .o-faq-panel-item__question button:before,
.t-card-accent-light .m-card .js-o-faq-panel .is-open .o-faq-panel-item__question button:before {
  background-image: var(--icon-arrow-up);
}

With this config:

const cssVariables = require('postcss-css-variables');

module.exports = {
  plugins: [
    cssVariables()
  ]
};

And I get this behavior. Running with postcss-preset-env makes this issue not happen.

@Antonio-Laguna
Copy link
Member

@nickyblissAviva forgot to add, you can test yourself here: https://madlittlemods.github.io/postcss-css-variables/playground/ (just found they have a playground)

@nickyblissAviva
Copy link
Author

nickyblissAviva commented Mar 26, 2022

Thanks. Looks like this is a "feature" of post-css-variables. Not sure why it suddenly uncovered during the upgrade but looks like we can either swap to the sister package postcss-custom-properties or use a cssnano option to remove to duplication (cssnano option looks a bit dangerous to use).

MadLittleMods/postcss-css-variables#58

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

3 participants