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

Problem since 1.40.0 with bootstrap 5 #1496

Closed
kris2kris opened this issue Sep 21, 2021 · 17 comments
Closed

Problem since 1.40.0 with bootstrap 5 #1496

kris2kris opened this issue Sep 21, 2021 · 17 comments

Comments

@kris2kris
Copy link

Hello,

The following code of bootstrap produce a build error since sass 1.40.0 (https://github.com/twbs/bootstrap/blob/main/scss/mixins/_grid.scss)

margin-top: calc(var(--#{$variable-prefix}gutter-y) * -1); // stylelint-disable-line function-disallowed-list
margin-right: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list
margin-left: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list

If I remove the minus sign before 1 or .5 it works. I'm not sure that multiplication should failed because of a minus sign.

@yudielcurbelo
Copy link

yudielcurbelo commented Sep 21, 2021

Same here.

Encapsulating the negative number in parenthesis as follows and everything works fine.

Original

margin-top: calc(var(--#{$variable-prefix}gutter-y) * -1); // stylelint-disable-line function-disallowed-list
margin-right: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list
margin-left: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list

Changed to

margin-top: calc(var(--#{$variable-prefix}gutter-y) * (-1)); // stylelint-disable-line function-disallowed-list
margin-right: calc(var(--#{$variable-prefix}gutter-x) * (-.5)); // stylelint-disable-line function-disallowed-list
margin-left: calc(var(--#{$variable-prefix}gutter-x) * (-.5)); // stylelint-disable-line function-disallowed-list

@nex3
Copy link
Contributor

nex3 commented Sep 21, 2021

I can't reproduce this with Dart Sass 1.42.0:

a {
  $variable-prefix: foo-;
  margin-top: calc(var(--#{$variable-prefix}gutter-y) * -1); // stylelint-disable-line function-disallowed-list
  margin-right: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list
  margin-left: calc(var(--#{$variable-prefix}gutter-x) * -.5); // stylelint-disable-line function-disallowed-list
}

This produces:

a {
  margin-top: calc(var(--foo-gutter-y) * -1);
  margin-right: calc(var(--foo-gutter-x) * -0.5);
  margin-left: calc(var(--foo-gutter-x) * -0.5);
}

as expected.

What version of Sass are you using? Can you provide a minimal stylesheet that reproduces the issue?

@kris2kris
Copy link
Author

kris2kris commented Sep 22, 2021

Hello @nex3
I tried with all versions from 1.40.0 to 1.42.0

For my test,

  • I create an app with create-react-app
  • I add bootstrap 5.1.1 and sass 1.42.0
  • import bootstrap from node_modules in a .scss file

This is the package.json file

{
  "name": "my-app",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "@testing-library/jest-dom": "^5.11.4",
    "@testing-library/react": "^11.1.0",
    "@testing-library/user-event": "^12.1.10",
    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-scripts": "4.0.3",
    "web-vitals": "^1.0.1",
    "bootstrap": "5.1.1",
    "sass": "1.42.0"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test",
    "eject": "react-scripts eject"
  },
  "eslintConfig": {
    "extends": [
      "react-app",
      "react-app/jest"
    ]
  },
  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  }
}

@bradbajuz
Copy link

Still having the issues with sass 1.42.1 and boostrap 5.1.1. Had to fall back to sass 1.39.2 to be able compile bootstrap 5.1.1

@maicol07
Copy link

I have the same issue with mwc checkbox:

PS C:\Users\Maicol\Documents\Progetti\Web\osm_rewrite> sass node_modules\.pnpm\@material+checkbox@12.0.0\node_modules\@material\checkbox\_checkbox-theme.scss
Error: Expected "(" or ".".
    ╷
329 │   $checkbox-padding: calc((_ripple-size - _icon-size) / 2);
    │                                        ^
    ╵
  node_modules\.pnpm\@material+checkbox@12.0.0\node_modules\@material\checkbox\_checkbox-theme.scss 329:40  root stylesheet

@stof
Copy link
Contributor

stof commented Sep 24, 2021

@maicol07 What are you expected to get for _ripple-size ? If you expect to use a variable, it misses a $.

@maicol07
Copy link

@maicol07 What are you expected to get for _ripple-size ? If you expect to use a variable, it misses a $.

That's not a file of mine. It's a file of MWC checkbox (a library I use in my project)

@stof
Copy link
Contributor

stof commented Sep 24, 2021

Well, if I look at the _checkbox-theme.scss file in the link you gave, it does not contain that source code at all...

@maicol07
Copy link

Yes, you're right, because that file belongs to a dependency of mwc checkbox: mdc checkbox.

Here is the related lines:

https://github.com/material-components/material-components-web/blob/65084baffaca256dd9eb77aae8fbafd379d8da00/packages/mdc-checkbox/_checkbox-theme.scss#L329

@stof
Copy link
Contributor

stof commented Sep 24, 2021

@maicol07 and they already fixed the issue in their code (but they haven't released it yet) when they were intended to use a string containing a template for a calc() for their JS code instead of a valid CSS calc computation.

@maicol07
Copy link

Oh, yeah, you're right. I haven't read the last commit

@stof
Copy link
Contributor

stof commented Sep 24, 2021

@maicol07 I suggest you close this issue then (I cannot do it myself, as I'm neither the author nor a maintainer, but only a contributor)

@maicol07
Copy link

@maicol07 I suggest you close this issue then (I cannot do it myself, as I'm neither the author nor a maintainer, but only a contributor)

I don't know if the OP has fixed his issue yet

@stof
Copy link
Contributor

stof commented Sep 24, 2021

@maicol07 ah yeah, I forgot that your reported your own issue in response to a different one.

@nex3
Copy link
Contributor

nex3 commented Sep 27, 2021

@kris2kris That isn't a minimal reproduction—I'm looking for a single Sass file without any dependencies that compiles to something unexpected. Nevertheless, I tried installing that version of bootstrap and running sass node_modules/bootstrap/scss/bootstrap.scss and it compiled successfully and produced reasonable-looking output.

@stof
Copy link
Contributor

stof commented Sep 28, 2021

I tried debugging this in create-react-app, and the error does not come from the Sass compiler at all.
It comes from postcss-values-parser (used by postcss) when trying to parse calc(var(--bs-gutter-y)*-1). and given that this is definetely valid CSS, it indicates a bug in postcss-values-parser

For reference, here is the stack trace I managed to get:
      ModuleBuildError: Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
      ModuleBuildError: Module build failed (from ./node_modules/postcss-loader/src/index.js):
      ParserError: Syntax Error at line: 1, column 25
          at /home/stof/tmp/dart_sass_1496/test/src/index.scss:6:8164
          at Parser.error (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-values-parser/lib/parser.js:127:11)
          at Parser.operator (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-values-parser/lib/parser.js:162:20)
          at Parser.parseTokens (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-values-parser/lib/parser.js:245:14)
          at Parser.loop (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-values-parser/lib/parser.js:132:12)
          at Parser.parse (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-values-parser/lib/parser.js:51:17)
          at parse (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-custom-properties/index.cjs.js:47:30)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/postcss-custom-properties/index.cjs.js:333:24
          at /home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:194:18
          at /home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:139:18
          at Rule.each (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:105:16)
          at Rule.walk (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:135:17)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:152:24
          at Root.each (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:105:16)
          at Root.walk (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:135:17)
          at Root.walkDecls (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss/lib/container.js:192:19)
          at transformProperties (/home/stof/tmp/dart_sass_1496/test/node_modules/postcss-custom-properties/index.cjs.js:330:8)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/NormalModule.js:316:20
          at /home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:367:11
          at /home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:233:18
          at context.callback (/home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/postcss-loader/src/index.js:208:9
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/NormalModule.js:316:20
          at /home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:367:11
          at /home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:182:20
          at context.callback (/home/stof/tmp/dart_sass_1496/test/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/mini-css-extract-plugin/dist/loader.js:148:14
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/Compiler.js:343:11
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/Compiler.js:681:15
          at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:15:1)
          at AsyncSeriesHook.lazyCompileHook (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/Hook.js:154:20)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/Compiler.js:678:31
          at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
          at AsyncSeriesHook.lazyCompileHook (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/Hook.js:154:20)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/Compilation.js:1423:35
          at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
          at AsyncSeriesHook.lazyCompileHook (/home/stof/tmp/dart_sass_1496/test/node_modules/tapable/lib/Hook.js:154:20)
          at /home/stof/tmp/dart_sass_1496/test/node_modules/webpack/lib/Compilation.js:1414:32

@nex3
Copy link
Contributor

nex3 commented Sep 28, 2021

Thanks for debugging, @stof! In that case, I'll close this down.

@nex3 nex3 closed this as completed Sep 28, 2021
ClementLmn added a commit to BagageStudio/vranken-pommery-monopole that referenced this issue Jan 12, 2022
danieleguido added a commit to C2DH/journal-of-digital-history that referenced this issue Dec 21, 2022
danieleguido added a commit to C2DH/journal-of-digital-history that referenced this issue Dec 22, 2022
* fixes double dep of babel loader and jest

* Update HomeReelItem.js

* fix build storybook - remove global bootstrap

* add dark mode

* Update package.json

$ NODE_OPTIONS=--openssl-legacy-provider build-storybook -s public --quiet
/opt/hostedtoolcache/node/16.18.1/x64/bin/node: --openssl-legacy-provider is not allowed in NODE_OPTIONS
error Command failed with exit code 9.

* remove automatic test temporarily

* downgrade sass to 1.39 following issue in dart sass

sass/dart-sass#1496

* Update storybook-test.yml

* Update storybook-test.yml

Co-authored-by: Daniele Guido <gui.daniele@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants