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: sass breaking changes #4528

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Lauwed
Copy link

@Lauwed Lauwed commented Jun 25, 2024

SASS new breaking changes about Strict Function Units are generating warnings and will become errors in few updates of SASS.

Changes

  • Removing NPM package node-sass as it is deprecated.
  • Installing sass ^1.77.6
  • Changed the rollup config Bundler.mjs to use the correct SASS compiler

To Do

  • Some deprecated warnings are not displayed, trying to find a way to show them.
  • Functions mergeColorMaps are not defined in ./src/scss/themes/bulma/variables.scss and not used. I put them in comments. See a how to solve the problem.
bundles ./src/scss/themes/bulma/tabulator_bulma.scss → dist/css/tabulator_bulma.css...
[!] (plugin postcss) Error: ("white": (hsl(0, 0%, 100%), hsl(0, 0%, 4%)), "black": (hsl(0, 0%, 4%), hsl(0, 0%, 100%)), "light": (hsl(0, 0%, 96%), hsl(0, 0%, 21%)), "dark": (hsl(0, 0%, 21%), hsl(0, 0%, 96%)), "primary": (hsl(171, 100%, 41%), findColorInvert(hsl(171, 100%, 41%))), "link": (hsl(217, 71%, 53%), findColorInvert(hsl(217, 71%, 53%))), "info": (hsl(204, 86%, 53%), findColorInvert(hsl(204, 86%, 53%))), "success": (hsl(141, 71%, 48%), findColorInvert(hsl(141, 71%, 48%))), "warning": (hsl(48, 100%, 67%), findColorInvert(hsl(48, 100%, 67%))), "danger": (hsl(348, 100%, 61%), findColorInvert(hsl(348, 100%, 61%)))) isn't a valid CSS value.
    ╷
128 │ $colors: mergeColorMaps(("white": ($white, $black), "black": ($black, $white), "light": ($light, $light-invert), "dark": ($dark, $dark-invert), "primary": ($primary, $primary-invert), "link": ($link, $link-invert), "info": ($info, $info-invert), "success": ($success, $success-invert), "warning": ($warning, $warning-invert), "danger": ($danger, $danger-invert)), $custom-colors) !default;
    │                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ value
    │          ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ unknown function treated as plain CSS
    ╵
  src\scss\themes\bulma\variables.scss 128:25     @import
  src\scss\themes\bulma\tabulator_bulma.scss 1:9  root stylesheet
src/scss/themes/bulma/tabulator_bulma.scss
    at Object.wrapException (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:2161:43)
    at C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:84868:25
    at _wrapJsFunctionForAsync_closure.$protected (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:4900:15)
    at _wrapJsFunctionForAsync_closure.call$2 (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:35592:12)
    at _awaitOnObject_closure0.call$2 (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:35586:25)
    at Object._rootRunBinary (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:5309:18)
    at StaticClosure.<anonymous> (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:115970:16)
    at _CustomZone.runBinary$3$3 (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:37049:39)
    at _FutureListener.handleError$1 (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:35809:21)
    at _Future__propagateToListeners_handleError.call$0 (C:\Users\Laura\Desktop\tabulator\node_modules\sass\sass.dart.js:36124:49)

@olifolkerd
Copy link
Owner

olifolkerd commented Jul 14, 2024

Hey @Lauwed

Thanks for the PR,

While i agree with the changes in pricipal, your PR make a large number of whitespace changes to the file that are not inline with the coding style of the library,

Please only submit functional changes in a PR rather than layout changes, otherwise it make it hard to detect the actual portions that have changed, and just results in layout battles between developers.

If you would be happy to resubmit with only the functional changes to the code base, i would be happy to look into accepting this

Cheers

Oli :)

@olifolkerd olifolkerd added the PR Needs Work The PR is a good idea, but needs some updates before it can be merged label Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Needs Work The PR is a good idea, but needs some updates before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants