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

sass-embedded error with bootstrap 5.3.3 #305

Closed
arnriu opened this issue Jun 17, 2024 · 13 comments
Closed

sass-embedded error with bootstrap 5.3.3 #305

arnriu opened this issue Jun 17, 2024 · 13 comments
Labels
bug Something isn't working needs info

Comments

@arnriu
Copy link

arnriu commented Jun 17, 2024

Since bootstrap 5.3.3, sass-embedded throw an error.
The problem was not present with bootstrap 5.3.2 and earlier, and is not present with normal sass package.

Here is the link to the open issue in bootstrap repo, with a reduced test case in a reproducible environment:
twbs/bootstrap#39978 (comment)

More specifically, the problem seems to be this line. If we remove it, everything runs fine:
https://github.com/twbs/bootstrap/blob/d2d4581790da2618d3fe063dafaa6205c73aabdd/scss/_variables.scss#L1751

@ntkme
Copy link
Contributor

ntkme commented Jun 17, 2024

I tested locally with a standalone setup without vite, both new API and legacy API worked just fine. This seems to be issue only when compiling via vite as pointed out by this comment: twbs/bootstrap#39978 (comment)

sass-embedded's emulated legacy API is indeed not fully compatible with sass in some corner cases. We can potentially look into what's the difference and try to fix it. However, at this point it would be much better for vite team to switch to the new API instead: vitejs/vite#7116

@nex3
Copy link
Contributor

nex3 commented Jun 17, 2024

Can you provide a minimal reproduction of this that doesn't involve a bunch of external dependencies?

@nex3 nex3 added bug Something isn't working needs info labels Jun 17, 2024
@arnriu
Copy link
Author

arnriu commented Jun 18, 2024

Sure.
Here it is without react, only vite + sass-embedded + bootstrap.

As a reminder:

  • in src/index.sass if you comment out @import bootstrap/scss/variables it will work. But if you let it, it will throw an error.
  • And in this file from the bootstrap package, variables it is the last line @import causing the problem. I didn't dig more.
  • if you switch from "sass": "npm:sass-embedded@1.77.1", to "sass": "npm:1.77.1", in package.json , run npm install and try again, it will work well as well. Problem only exists with sass-embedded.

https://codesandbox.io/p/devbox/competent-mahavira-forked-3jh7fj?file=%2Fpackage.json

@nex3
Copy link
Contributor

nex3 commented Jul 11, 2024

I'm looking for a reproduction without vite as well—for all we know, this is a bug in Vite's custom importer.

@Digital-Pig-LLC
Copy link

Was this resolved, or just closed?
I'm running the latest version of this and trying to compile Bootstrap v5.3.2 and v5.3.3 I get the same 30 issues.

Unresolvable

Cannot resolve '--bs-body-text-align' custom property
Unknown pseudo selector '-webkit-datetime-edit'
Cannot resolve '--bs-dropdown-item-border-radius' custom property
Cannot resolve '--bs-nav-link-font-size' custom property
Cannot resolve '--bs-scroll-height' custom property
Cannot resolve '--bs-breadcrumb-font-size' custom property
Cannot resolve '--bs-breadcrumb-divider' custom property
Cannot resolve '--bs-focus-ring-x' custom property
Mismatched parameters ( [, {2,4}]?)
Cannot resolve '--bs-focus-ring-y' custom property
Mismatched parameters ( [, {2,4}]?)
Cannot resolve '--bs-focus-ring-blur' custom property
Mismatched parameters ( [, {2,4}]?)
Cannot resolve '--bs-icon-link-transform' custom property

Missing values

--bs-btn-font-family: ;
--bs-dropdown-box-shadow: ;
--bs-nav-link-font-weight: ;
--bs-nav-link-font-weight: ;
--bs-card-title-color: ;
--bs-card-subtitle-color: ;
--bs-card-box-shadow: ;
--bs-card-cap-color: ;
--bs-card-height: ;
--bs-card-color: ;
--bs-breadcrumb-bg: ;
--bs-breadcrumb-border-radius: ;
--bs-toast-color: ;
--bs-modal-color: ;
--bs-modal-footer-bg: ;
--bs-tooltip-margin: ;

@Digital-Pig-LLC
Copy link

Info
##########
sass-embedded@1.77.8 | MIT | deps: 6 | versions: 61
Node.js library that communicates with Embedded Dart Sass using the Embedded Sass protocol
https://github.com/sass/embedded-host-node#readme

dist
.tarball: https://registry.npmjs.org/sass-embedded/-/sass-embedded-1.77.8.tgz
.shasum: d8d885ccd59c6040fcccd345299a115187d65726
.integrity: sha512-WGXA6jcaoBo5Uhw0HX/s6z/sl3zyYQ7ZOnLOJzqwpctFcFmU4L07zn51e2VSkXXFpQZFAdMZNqOGz/7h/fvcRA==
.unpackedSize: 744.2 kB

dependencies:
@bufbuild/protobuf: ^1.0.0 immutable: ^4.0.0 supports-color: ^8.1.1
buffer-builder: ^0.2.0 rxjs: ^7.4.0 varint: ^6.0.0

maintainers:

dist-tags:
latest: 1.77.8

@nex3
Copy link
Contributor

nex3 commented Aug 30, 2024

@Digital-Pig-LLC none of those errors are coming from Sass.

@Digital-Pig-LLC
Copy link

No, they appear in the compiled CSS which is compiled from bootstraps SCSS files using sass-embedded@1.77.8.
What I did notice is that for all of the missing values there is a null value assigned in the bootstrap SCSS file.
For example, in the SCSS there will be: --bs-btn-font-family: null !default; and that will result in an empty value upon compilation.

I ran the request as follows so ensure it wasn't my PHPStorm that was causing the issue:
.\node_modules\sass-embedded-win32-x64\dart-sass\sass.bat ......\PhpstormProjects\vTribes\2.0-php82\src\vg_config\scss\cpnl.scss ......\PhpstormProjects\vTribes\2.0-php82\src\public_html\css\cpnl.css

My cpnl.scss has an import from root.scss, which is where the imports for bootstrap exist.

A very simple case of root.scss:
:root {
--app-width: 1000px;
--app-cp-width: 1200px;
--app-bg-light: #909dba;
--app-bg-dark: #7b849a;
--app-accent: #1c3744;
}

@import "../../node_modules/bootstrap/scss/bootstrap";

and in my cpnl.scss:
@import "root";

The resulting CSS file ends up with 30 errors, as shown above, regardless of using bootstrap v5.3.2 or 5.3.3 as source.

@nex3
Copy link
Contributor

nex3 commented Aug 31, 2024

I'm not sure what you mean by an error "appearing in" compiled CSS. CSS itself doesn't produce any errors—only tools that consume it.

If you have a specific case where Embedded Sass is producing the wrong output, especially if you can produce a minimal reproduction of that case, I can look into it. But this doesn't really give me any information about what Sass is doing or what behavior you expect.

@Digital-Pig-LLC
Copy link

Ok, maybe I'm not using correct terminology or maybe we're just not understanding each other.
I have the source SCSS files from Bootstrap. When I try to compile them using sass-embedded, the output CSS files are missing values, making them invalid.
When looking at the missing values in _variables.scss I see that they are marked as: null !default

Surely if this were a global problem with all compilers, there would be a lot of invalid CSS and Bootstrap would make us aware...how did we even get to 5.3.3 as stable if this were the case.

Uncompiled -> sass-embedded -> compiled...is that not the workflow?

Please correct me if I'm wrong in any of this. I'm certainly no SCSS wizard

@nex3
Copy link
Contributor

nex3 commented Sep 1, 2024

What is the CSS output you expect, the CSS output you're actually getting, and the Sass that produces it?

@julien-deramond
Copy link

For embedded-host-node maintainers, I hope I don't overstep, feel free to hide this message if it generates noise :)


Hey @Digital-Pig-LLC, one of the Bootstrap maintainers here.
Based on what I can see in your #305 (comment) message, the warnings/errors are related to twbs/bootstrap#36595, twbs/bootstrap#38474, and many other duplicates in our Bootstrap repository.
These warnings/errors are not related to Sass, and are not related to this embedded-host-node issue.
They are related to Bootstrap, and are not really issues but development choices, even if we'll try to improve the situation by dropping some of them from the generated CSS file.

@nex3
Copy link
Contributor

nex3 commented Sep 3, 2024

I'm going to close this out since the original reporter has not provided further information for over a month.

@nex3 nex3 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs info
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants