Semantic-Org/Semantic-UI-LESS

How to resolved this Error: the first argument to unit must be a number

ParthS007 opened this issue ยท 13 comments

./node_modules/semantic-ui-less/semantic.less (./node_modules/css-loader/dist/cjs.js??ref--5-oneOf-9-1!./node_modules/postcss-loader/src??postcss!./node_modules/less-loader/dist/cjs.js??ref--5-oneOf-9-3!./node_modules/semantic-ui-less/semantic.less)

@relativeLarge   : unit( round(@largeSize * @emSize) / @emSize, em);
@relativeBig     : unit( round(@bigSize * @emSize) / @emSize, em);
                 ^
Error evaluating function `unit`: the first argument to unit must be a number. Have you forgotten parenthesis?
      Error in .. /node_modules/semantic-ui-less/themes/default/globals/site.variables (line 408, column 19)

Started getting the exact same error today. Was working yesterday. Running the same CI/CD pipeline job on the exact same commit went from working to failing.

Using module version 2.4.1

@ParthS007 do you have the craco-less module in your dependencies? I noticed that there was a new version (1.18.0) released today and that seems to coincide with the start of this error for us. Since this is a dependency of module @semantic-ui-react/craco-less I don't have direct control over the version of craco-less being installed, and I'm not having any luck with npm honoring my package-lock.json change to force craco-less to version 1.17.1 that being used before this error started.

@joshua-phillips I have the craco-less in the dependencies and yes, the latest version seems to cause the error. I will post the error there as well.

Here's how we managed to pin the craco-less in package.json, thanks to @mvidalgarcia for the fix.

...
"resolutions": {
    "@semantic-ui-react/craco-less/craco-less": "1.17.1"
   }
...

Hope it helps you as well. :)

Thanks, @ParthS007 ! I was able to verify that resolved my build failure as well. It looks like npm does not support the resolutions object in package.json, but there is a node module that adds support when using npm. I haven't tested it yet though, but just thought I'd mention it here in case anyone else comes across this issue and they're using npm instead of yarn.

https://www.npmjs.com/package/npm-force-resolutions

Thanks for the resolution fix @ParthS007 and @joshua-phillips ! Still getting the same error when using npm and the npm-force-resolutions with that resolutions path. Have tried deleting node_modules and running a fresh install, as well as changing the preinstall script to npm install --package-lock-only --ignore-scripts && npx npm-force-resolutions-- any ideas?

@Alexa-Green we found that same thing unfortunately that the npm-force-resolutions module was not working and we ended up switching to using yarn. Although I'm curious why with that module it's not working with npm so I'll probably play around with it some more later. If I figure anything out, I'll post back here

@joshua-phillips Switched to yarn as well after some digging into npm-force-resolutions with no results-- yarn (/resolutions support) helped with some other issues that were going on too so it all works in the meantime! Curious to hear if you discover anything

craco-less version 1.18.0 now depends on "less": "^4.1.1" instead of 3.x. According to lesscss some math options changed by default, e.g., "in order to cause fewer conflicts with CSS, which now liberally uses the / symbol between values, there is now a math mode that only requires parentheses for division. (This is now the default in Less 4.)".

To keep semantic-ui-less running again, the math option has to be changed back to always, e.g., for webpack:

    "loader": "less-loader",
        "options": {
            "lessOptions": {
                "math": "always"
            }
    }

or using @semantic-ui-react/craco-less:

    plugin: require("@semantic-ui-react/craco-less"),
    options: {
        lessLoaderOptions: {
            lessOptions: {
                math: "always"
            }
        }
    }

I confirm what @kenomo mentioned. My craco.config.js looks like this:

module.exports = {
  plugins: [
    {
      plugin: require("@semantic-ui-react/craco-less"),
      options: { lessLoaderOptions: { lessOptions: { math: "always" } } },
    }
  ],
};

This solves the problem as a workaround, however, I guess this should be changed at @semantic-ui-react/craco-less plugin level, namely here? Otherwise all the less files among the different themes coming from Semantic-UI-LESS will face the same kind of issues?

I think something like this in semantic-ui-tools should work:

diff --git a/packages/craco-less/src/index.js b/packages/craco-less/src/index.js
index ebafeb0..dfbffb8 100644
--- a/packages/craco-less/src/index.js
+++ b/packages/craco-less/src/index.js
@@ -8,7 +8,7 @@ const CracoLessPlugin = require("craco-less");
 const path = require("path");
 
 const overrideWebpackConfig = ({ context, pluginOptions, webpackConfig }) => {
-  pluginOptions = pluginOptions || {};
+  pluginOptions = {lessLoaderOptions: { lessOptions: { math: 'always' } }, ...pluginOptions};
 
   // add alias to theme.config
   webpackConfig.resolve.alias["../../theme.config$"] = path.join(

@layershifter I can create a PR if needed, let me know.

@layershifter I can create a PR if needed, let me know.

Yes, please ๐Ÿ™

Confirmed that updating to @semantic-ui-react/craco-less@1.2.3 has gotten rid of this error with no need to update craco.config.js or add a resolution to package.json -- thank you @mvidalgarcia !

Awesome! Thanks, everyone :)