Skip to content

Fix dart-sass deprecation warning#39030

Merged
mdo merged 3 commits into
twbs:mainfrom
blankse:blankse-patch-1
Sep 12, 2023
Merged

Fix dart-sass deprecation warning#39030
mdo merged 3 commits into
twbs:mainfrom
blankse:blankse-patch-1

Conversation

@blankse

@blankse blankse commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown

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

@blankse

blankse commented Aug 10, 2023

Copy link
Copy Markdown
Contributor Author

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.

@louismaximepiton louismaximepiton left a comment

Copy link
Copy Markdown
Member

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

flyke commented Aug 16, 2023

Copy link
Copy Markdown

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
Copy Markdown

Bootstrap v4.6 has the same issue

@XhmikosR

Copy link
Copy Markdown
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.

@julien-deramond julien-deramond self-requested a review August 24, 2023 06:21

@julien-deramond julien-deramond left a comment

Copy link
Copy Markdown
Member

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

julien-deramond commented Aug 31, 2023

Copy link
Copy Markdown
Member

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

XhmikosR commented Aug 31, 2023 via email

Copy link
Copy Markdown
Member

@julien-deramond

Copy link
Copy Markdown
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
@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
Copy Markdown

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

@julien-deramond

Copy link
Copy Markdown
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.