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.

Sign up for GitHub

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

Jump to bottom

Start compatibility work on SASS 2 #40864

Closed
wants to merge 2 commits into from
Closed

Start compatibility work on SASS 2 #40864

wants to merge 2 commits into from

Conversation

thet
Copy link

@thet thet commented Sep 21, 2024 •
edited
Loading

Description

Fixes deprecation warnings starting in SASS 1.79 and fixes #40849

Motivation & Context

Plone is using Twitter Bootstrap with it's default theme. When doing JavaScript work, webpack presents a big warning and breaks execution of any other code.
We will silence the warnings as described in https://sass-lang.com/documentation/breaking-changes/legacy-js-api/#silencing-warnings but for progressing, this issue needs to be fixed anyways.

Type of changes

It changes the usage of the deprecated red, green and blue converter functions.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Tests do not pass. Therefore this PR is in draft mode. Help wanted.

Live previews

There are no documentation changes.
Here is a Preview to the SASS playground where I tested my changes:

https://sass-lang.com/playground/#eJyVj0EKwyAQRfeeYpAEFCQYSheNm1xF46ALqzDW+1fTRbPNapjPf4+Zdd1bReDV1rodJRXihl2it/3EnrCJ0G8wtoVKy16c3eWINmdMItYkXtpjgIee4alnqYB3hCug4KQ0bAqEmO8qTugicanhXcdg/gq2e3QtwHhIwe+qPkfJfAFc21KF

Related issues

#40849

@thet
Compatibility with SASS version 2.
b22f08f 
Fixing deprecation warnings for usage of red, green and blue functions and using color.channel instead.

Fixes: twbs#40849
@thet
npm install.
77b12a3
@thet thet requested a review from a team as a code owner September 21, 2024 11:27
@thet thet marked this pull request as draft September 21, 2024 11:27
@julien-deramond
Copy link
Member

julien-deramond commented Sep 21, 2024 •
edited
Loading

Thanks for trying something for this issue @thet
Unfortunately, Bootstrap v5 still need to support node-sass too. So using sass:math nor sass:color won't probably be possible. This is why this issue will be tricky IMO.

To test locally the node-sass compilation of Bootstrap: npx --package node-sass@8 node-sass --output-style expanded --source-map true --source-map-contents true --precision 6 scss/ -o dist-sass/css/

In this PR, the error is:

{
  "status": 1,
  "file": "/Users/ju/twbs/bootstrap/scss/_functions.scss",
  "line": 40,
  "column": 13,
  "message": "Invalid CSS after \"  $red: math\": expected expression (e.g. 1px, bold), was \".round(color.channe\"",
  "formatted": "Error: Invalid CSS after \"  $red: math\": expected expression (e.g. 1px, bold), was \".round(color.channe\"\n        on line 40 of scss/_functions.scss\n        from line 6 of scss/bootstrap-grid.scss\n>>   $red: math.round(color.channel($value, \"red\", rgb));\n\n   ------------^\n"
}

@thet
Copy link
Author

thet commented Sep 21, 2024

Ah, node-sass is giving that error.
Thanks for pointing out!

@julien-deramond
Copy link
Member

Thanks for your work on this! I'm going to close it for now to help keep our backlog manageable. Feel free to reopen it if you come up with a different or new solution :)

@julien-deramond julien-deramond mentioned this pull request Sep 22, 2024
Closed
10 tasks
@thet thet mentioned this pull request Sep 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New deprecations in Sass 1.79
2 participants
@thet @julien-deramond