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

Fix dart-sass deprecation warning #39030

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Fix dart-sass deprecation warning #39030

merged 3 commits into from
Sep 12, 2023

Conversation

blankse
Copy link
Contributor

@blankse blankse commented Aug 10, 2023

Description

Fix the deprecation warning with dart-sass. See #39028

Type of changes

  • 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

Related issues

Fixes #39028

@MatthiasPeltzer
Copy link

hi... isn´t it more like percentage(divide(1, $count))? cause 100 gives eg. 333.33333% back.

@blankse
Copy link
Contributor Author

blankse commented Aug 10, 2023

hi... isn´t it more like percentage(divide(1, $count))? cause 100 gives eg. 333.33333% back.

Yes you are right. I changed it ad76686

Now the percentages are correct.

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thanks for the contribution!

@flyke
Copy link

flyke commented Aug 16, 2023

So to use Bootstrap in a project including this pull request, you can either use:
npm install --save-dev twbs/bootstrap#39030/head
or:
yarn add --dev twbs/bootstrap#39030/head
(remove --save-dev or --dev if you want to add it as a dependency rather than a dev dependency)

@XhmikosR XhmikosR requested a review from mdo August 16, 2023 13:59
@ankurk91
Copy link

Bootstrap v4.6 has the same issue

@XhmikosR
Copy link
Member

@twbs/css-review we might also need to backport the fix in the rfs repo. If someone fixes the issue there, please CC me.

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

16.6666666667%; becomes 16.66666667%; and 33.3333333333%; becomes 33.33333333%; but this precision is enough (already used in bootstrap.css in other contexts).

@julien-deramond
Copy link
Member

julien-deramond commented Aug 31, 2023

Another possibility is the following tested by @louismaximepiton (https://github.com/twbs/bootstrap/compare/main-lmp-abs-warning-fix + fix in twbs/rfs) which would make it transparent even for folks using divide() in their own context/project.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 31, 2023 via email

@julien-deramond
Copy link
Member

We tried something via #39132 but fixing divide() by handling the dividend unit doesn't seem compatible with node-sass we still maintain (see #39132 (comment)).

I suggest that we merge + ship this patch. And then, if we get rid of the node-sass compatibility in v6, go with a solution based on what's in #39132.

It means that there would be nothing to do for now in twbs/rfs.

@twbs/css-review all fine with this proposal? Thoughts?

@mdo mdo merged commit d07d3a6 into twbs:main Sep 12, 2023
13 checks passed
@blankse blankse deleted the blankse-patch-1 branch September 12, 2023 20:14
webdevian added a commit to BundleStars/bootstrap that referenced this pull request Sep 13, 2023
@n-rodriguez
Copy link

@julien-deramond any chance to see this patch backported in BS 4.x?

@julien-deramond
Copy link
Member

@n-rodriguez Bootstrap 4 has been EOL since 2023-01-01, so unfortunately there won't be a patch backported to it.
I haven't tested it but if you're using Sass, there's prolly a way to override the part that generates the deprecation warning on your side.

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

Successfully merging this pull request may close these issues.

dart-sass deprecation warning: Passing percentage units to the global abs() function is deprecated.