search

carbon-design-system / carbon-addons-iot-react

A collection of React components shared between IBM Watson IoT products.
https://carbon-design-system.github.io/carbon-addons-iot-react/
Apache License 2.0
96 stars 77 forks source link

[ComboBox] Negating CSS variables in `calc()` breaks OptimizeCSSAssetsPlugin #2314

Closed TheSharpieOne closed 3 years ago

TheSharpieOne commented 3 years ago

What package is this for?

Describe the bug

negating a var within calc breaks OptimizeCSSAssetsPlugin (which is used by CRA in production builds) with

Failed to compile.

Parse error on line 1: 
-var(--cds-...
-^
Expecting "ADD", "SUB", "LENGTH", "ANGLE", "TIME", "FREQ", "RES", "UNKNOWN_DIMENSION", "EMS", "EXS", "CHS", "REMS", "VHS", "VWS", "VMINS", "VMAXS", "PERCENTAGE", "NUMBER", "dimension", got unexpected "FUNCTION"
CompileError: Begins at CSS selector undefined

Tracking down the error (which was not easy) finds the following line: https://github.com/carbon-design-system/carbon-addons-iot-react/blob/c0328c3869cd9b2ecafbc13b3c04f63114e8a8db/packages/react/src/components/ComboBox/_combo-box.scss#L32 This is just the first of many in this particular file (and probably the entire project). This particular line end up being interpolated into bottom: calc(-var(--cds-spacing-08, 2.5rem)) since $ai-apps-combobox-input-height is $spacing-08 and $spacing-08 is var(--cds-spacing-08, 2.5rem).

The best way to handle negating the value is to multiple by -1: https://github.com/css-modules/postcss-icss-values/issues/64#issuecomment-241285628

To Reproduce

Steps to reproduce the behavior:

  1. create a new app with CRA
  2. add ComboBox styles
  3. run the production build
  4. See error

cannot be recreated using codesandbox since it happens during production build

Expected behavior

No error is given and the production build produces the correct code.

Environment/versions:

Additional context

Current workaround, use CRACO remove / disable OptimizeCSSAssetsPlugin (which is not that great...)

Specific timeline issues / requests

N/A

kevinsperrine commented 3 years ago

@TheSharpieOne I'm having trouble recreating this one. Can you give me some details on what versions you were using, so that I can recreate the problem fully and find all instances where our code is causes these breaks?

This is what I've done to attempt to recreate it:

  1. npx create-react-app pal-test
  2. yarn add carbon-addons-iot-react@next d3@5.14.2 node-sass monaco-editor
  3. rename App.css to App.scss
  4. Clear contents of App.scss and add @import '~carbon-addons-iot-react/scss/globals/vars'; and @import "~carbon-addons-iot-react/scss/components/ComboBox/combo-box";
  5. Add a minimal ComboBox to App.js 
    <div style={{ width: 400 }}>
<ComboBox id="combobox" items={[{id: '0', text: 'Option 1'}, {id: '1', text: 'Option 2'}]} itemToString={(item) => (item ? item.text : '')} label="Combox test" placeholder="Test items" onChange={(...args) => console.log(...args)} />
</div>
  6. run yarn build

Everything builds and runs fine with these versions:

react-scripts@4.0.3:
  version "4.0.3"
  resolved "https://registry.yarnpkg.com/react-scripts/-/react-scripts-4.0.3.tgz#b1cafed7c3fa603e7628ba0f187787964cb5d345"
  integrity sha512-S5eO4vjUzUisvkIPB7jVsKtuH2HhWcASREYWHAQ1FP5HyCv3xgn+wpILAEWkmy+A+tTNbSZClhxjT3qz6g4L1A==
  dependencies:
    "@babel/core" "7.12.3"
    "@pmmmwh/react-refresh-webpack-plugin" "0.4.3"
    "@svgr/webpack" "5.5.0"
    "@typescript-eslint/eslint-plugin" "^4.5.0"
    "@typescript-eslint/parser" "^4.5.0"
    babel-eslint "^10.1.0"
    babel-jest "^26.6.0"
    babel-loader "8.1.0"
    babel-plugin-named-asset-import "^0.3.7"
    babel-preset-react-app "^10.0.0"
    bfj "^7.0.2"
    camelcase "^6.1.0"
    case-sensitive-paths-webpack-plugin "2.3.0"
    css-loader "4.3.0"
    dotenv "8.2.0"
    dotenv-expand "5.1.0"
    eslint "^7.11.0"
    eslint-config-react-app "^6.0.0"
    eslint-plugin-flowtype "^5.2.0"
    eslint-plugin-import "^2.22.1"
    eslint-plugin-jest "^24.1.0"
    eslint-plugin-jsx-a11y "^6.3.1"
    eslint-plugin-react "^7.21.5"
    eslint-plugin-react-hooks "^4.2.0"
    eslint-plugin-testing-library "^3.9.2"
    eslint-webpack-plugin "^2.5.2"
    file-loader "6.1.1"
    fs-extra "^9.0.1"
    html-webpack-plugin "4.5.0"
    identity-obj-proxy "3.0.0"
    jest "26.6.0"
    jest-circus "26.6.0"
    jest-resolve "26.6.0"
    jest-watch-typeahead "0.6.1"
    mini-css-extract-plugin "0.11.3"
    optimize-css-assets-webpack-plugin "5.0.4"
    pnp-webpack-plugin "1.6.4"
    postcss-flexbugs-fixes "4.2.1"
    postcss-loader "3.0.0"
    postcss-normalize "8.0.1"
    postcss-preset-env "6.7.0"
    postcss-safe-parser "5.0.2"
    prompts "2.4.0"
    react-app-polyfill "^2.0.0"
    react-dev-utils "^11.0.3"
    react-refresh "^0.8.3"
    resolve "1.18.1"
    resolve-url-loader "^3.1.2"
    sass-loader "^10.0.5"
    semver "7.3.2"
    style-loader "1.3.0"
    terser-webpack-plugin "4.2.3"
    ts-pnp "1.2.0"
    url-loader "4.1.1"
    webpack "4.44.2"
    webpack-dev-server "3.11.1"
    webpack-manifest-plugin "2.2.0"
    workbox-webpack-plugin "5.1.4"
  optionalDependencies:
    fsevents "^2.1.3"

node-sass@^6.0.1:
  version "6.0.1"
  resolved "https://registry.yarnpkg.com/node-sass/-/node-sass-6.0.1.tgz#cad1ccd0ce63e35c7181f545d8b986f3a9a887fe"
  integrity sha512-f+Rbqt92Ful9gX0cGtdYwjTrWAaGURgaK5rZCWOgCNyGWusFYHhbqCCBoFBeat+HKETOU02AyTxNhJV0YZf2jQ==
  dependencies:
    async-foreach "^0.1.3"
    chalk "^1.1.1"
    cross-spawn "^7.0.3"
    gaze "^1.0.0"
    get-stdin "^4.0.1"
    glob "^7.0.3"
    lodash "^4.17.15"
    meow "^9.0.0"
    nan "^2.13.2"
    node-gyp "^7.1.0"
    npmlog "^4.0.0"
    request "^2.88.0"
    sass-graph "2.2.5"
    stdout-stream "^1.4.0"
    "true-case-path" "^1.0.2"
JordanWSmith15 commented 3 years ago

@TheSharpieOne Friendly reminder... can you answer Kevin's question?

TheSharpieOne commented 3 years ago

Sorry, I was out for a bit. It seems that this is triggered by importing ~@console/pal/scss/theme first. We are using PAL, and the PAL theme seems to set some stuff up that causes this issue.

I am not sure if we are supposed to be using the PAL theme with this package or not. PAL is setting $spacing-08 which is being used by this package in the place that is breaking.

importing just ~@console/pal/scss/theme works fine. importing just ~carbon-addons-iot-react/scss/<stuff> works fine. Importing both (with pal first since it is setting vars used by carbon-addons-iot-react/scss) causes carbon-addons-iot-react/scss to break.

scss:

@import '~@console/pal/scss/theme';
@import '~carbon-addons-iot-react/scss/globals/vars';
@import "~carbon-addons-iot-react/scss/components/ComboBox/combo-box";
kevinsperrine commented 3 years ago

@TheSharpieOne Thank you! Where do you get the @console/pal package? I'm unfamiliar with it, but I do see references to it in other carbon issues.

TheSharpieOne commented 3 years ago

It's available via IBM's artifactory. More information: https://github.ibm.com/console-pipeline/docs/tree/master/migrating-to-artifactory#instructions https://github.ibm.com/ibmcloud/pal#how-to-install-and-use