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

Prevent null Sass variables from creating empty CSS variables #36595

Open
3 tasks done
Kiriakk opened this issue Jun 16, 2022 · 10 comments
Open
3 tasks done

Prevent null Sass variables from creating empty CSS variables #36595

Kiriakk opened this issue Jun 16, 2022 · 10 comments
Labels

Comments

@Kiriakk
Copy link

Kiriakk commented Jun 16, 2022

Prerequisites

Describe the issue

I'm trying to compile bootstrap's scss without any changes. It compiles fine, but produces broken css with 15 absent values.
For example this:
--bs-heading-color: ; at line 71

Compiling with Dart-Sass 1.52.3

Actually, already compiled css, which ships inside bootstrap-5.2.0-beta1.zip and latest bootstrap-main.zip files contain that broken css as well.

Scss from 5.1.3 compiles proper css on the same setup.

Reduced test cases

Try to compile with minimal custom.scss file:
@import "../node_modules/bootstrap/scss/bootstrap";

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome, Opera

What version of Bootstrap are you using?

5.2.0 beta 1, latest snapshot.

@mdo
Copy link
Member

mdo commented Jun 16, 2022

This isn't exactly broken, it's that the Sass variables are null and do not compile to anything. This works great in our normal property usage (Sass will remove the property: null line during compilation), but not so well with CSS variables. We may need to add a manual check for these things in the future.

#35442 aimed to try that out, but I wasn't too keen on the extra syntax on top of each instance.

@ffoodd
Copy link
Member

ffoodd commented Jun 17, 2022

FWIW, this is valid CSS. 🤷

The only potential issue is the predictability regarding the behaviour, since every property will inherit and some won't (but again, this is the standard behaviour when you don't provide any value).

Dropping them would decrease file size a little bit, but I'm not sure it'd be noticeable.

@Kiriakk
Copy link
Author

Kiriakk commented Jun 17, 2022

FWIW, this is valid CSS. 🤷

The only potential issue is the predictability regarding the behaviour, since every property will inherit and some won't (but agai', this is thé standard behaviour when you don't provide any value).

Dropping them would decrease file size a little bit, but I'm not sure it'd be noticeable.

My biggest issue here is that compiled css is considered broken by my IDE due to the missing values and subsequent minify task is not being run because of this.

@ffoodd
Copy link
Member

ffoodd commented Jun 17, 2022

Then those are bugs in IDE, linters and minifiers.

Nevertheless, those empty custom properties are useless and should be dropped to prevent confusion and save a few bytes.

@mdo mdo added this to v5.2.1 Jul 19, 2022
@mdo mdo changed the title 5.2.0 beta 1 scss produces broken css with missing values Prevent null Sass variables from creating empty CSS variables Jul 19, 2022
@mdo mdo moved this to Todo in v5.2.1 Jul 19, 2022
@julien-deramond julien-deramond mentioned this issue Aug 24, 2022
3 tasks
@rbsdotnet

This comment was marked as off-topic.

@mdo mdo removed this from v5.2.1 Sep 1, 2022
@mdo mdo added this to v5.3.0 Sep 1, 2022
@mdo mdo moved this to To do in v5.3.0 Sep 1, 2022
@rbsdotnet

This comment was marked as off-topic.

@mdo
Copy link
Member

mdo commented Sep 10, 2022

Hi Dear @julien-deramond

When will this problem be solved?

When the issue is closed, you'll know it's been addressed ;).

@Yousha
Copy link

Yousha commented Jan 1, 2024

Missing property values in the "(property) : (value)" declaration.
Using compiled Bootstrap 5.2.3 in file bootstrap.css:

bootstrap.css -> Line: 4194	
bootstrap.css -> Line: 2807	
bootstrap.css -> Line: 3532	
bootstrap.css -> Line: 3646	
bootstrap.css -> Line: 3820	
bootstrap.css -> Line: 4199	
bootstrap.css -> Line: 4200	
bootstrap.css -> Line: 4201	
bootstrap.css -> Line: 4508	
bootstrap.css -> Line: 4509	
bootstrap.css -> Line: 5210	
bootstrap.css -> Line: 5277	
bootstrap.css -> Line: 5291	
bootstrap.css -> Line: 5569	
bootstrap.css -> Line: 5680	
bootstrap.css -> Line: 6096	

@tlsimmons88
Copy link

This is still an issue in Bootstrap 5.3.2

I'm trying to use less and it is failing due to the value being empty.

@SilvanVerhoeven
Copy link

SilvanVerhoeven commented Feb 14, 2024

Are there any updates or known fixes on this? I am also compiling Bootstrap 5.3.2 using LibSass and .nav-link breaks because --bs-nav-link-padding-y and --bs-nav-link-padding-x are not set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: To do
Development

No branches or pull requests

8 participants