Polaris-react: Failed to compile ./node_modules/@shopify/polaris/styles.css

[mbaumbach]

Issue summary

When using a standard create-react-app template and attempting to create a production build, you get a compile error with the Polaris styles.css file.

Expected behavior

No errors when running a production build.

Actual behavior

Running a yarn build (or the npm equivalent) on a standard create-react-app using the latest Polaris (4.11.0) you get the following error:

Creating an optimized production build...
Failed to compile.

./node_modules/@shopify/polaris/styles.css
ParserError: Syntax Error at line: 1, column 34


error Command failed with exit code 1.

This appears to be due to the browserslist definition for the production build, though this worked in previous versions of Polaris (<= 4.10.2). This will also break on development builds in apps generated with older versions of create-react-app. More recent create-react-app use different browserslist definitions between the product and development build in the package.json:

```package.json
"browserslist": {
"production": [
">0.2%",
"not dead",
"not op_mini all"
],
"development": [
"last 1 chrome version",
"last 1 firefox version",
"last 1 safari version"
]
}

If you copy the array of values from `production` into the `development` key, you'll be able to reproduce it in development when simply running `yarn start`. Regardless, this breaks on all versions of `create-react-app` when doing a production build.

## Steps to reproduce the problem

1. Create a standard create-react-app: `yarn create react-app testing`
1. Add the latest version of Polaris (4.11.0): `yarn add @shopify/polaris`
1. Install the style script at the top of the `index.js` file as specified by the Polaris install docs.
1. Run a production build: `yarn build`

## Reduced test case

I can't seem to create a codesandbox that reproduces it, but the App.js can be any standard Polaris app:

```jsx
import React from 'react';
import enTranslations from '@shopify/polaris/locales/en.json';
import { AppProvider, Page, Card, Button } from '@shopify/polaris';

function App() {
    return (
        <AppProvider i18n={enTranslations}>
            <Page title="Example app">
                <Card sectioned>
                    <Button onClick={() => alert('Button clicked!')}>Example button</Button>
                </Card>
            </Page>
        </AppProvider>
    );
}

export default App;

Specifications

• Are you using the React components? (Y/N): Y
• Polaris version number: 4.11.0
• Browser: None needed
• Device: MacBook Pro
• Operating System: macOS Catalina 10.15.2

[GrandNewbien]

Who pushed this change with zero oversight?

Do they not have any tests in place at all?

[BPScott]

After a highly technical debugging process of working my way through node_modules/@shopify/polaris/styles.css and uncommenting chunks of the file and seeing if it can compile or not I've dug down to the minimal change required

It seems in two places (Checkbox global theming and RadioButton global theming) we do the following:

    top:calc(var(--control-border-width)*-1);
    right:calc(var(--control-border-width)*-1);
    bottom:calc(var(--control-border-width)*-1);
    left:calc(var(--control-border-width)*-1);

Commenting both occurrences of that out results in a build that works.

You can recreate this build failure in an otherwise pristine CRA app by adding the following to index.css

.thing{
  --control-border-width:0.2rem;
}

.thing::before{
    top:calc(var(--control-border-width)*-1);
 }

Something in CRA's css processing (probably css-preset-env, probably https://github.com/postcss/postcss-custom-properties) doesn't like the idea of that css variable in a calc. I'll see If i can shrink this root cause down even further. post-css-env bugs have bit us before.

Pinging @dleroux as he knows more about the globalTheming stuff than I. The source of this in our code is a mixin in _controls.scss: https://github.com/Shopify/polaris-react/blob/master/src/styles/shared/_controls.scss#L113-L116. I'm not sure there's a good way to change this yet short of a temporary revert pending an upstream fix but FYI.

---

In the mean time you can also fix this by making it so that custom-properties transform doesn't trigger by slightly increasing your production browserslist coverage: The following compiles

      ">0.3%",
      "not dead",
      "not ie 11",
      "not safari 5.1",

Compare https://browsersl.ist/?q=%3E+0.2%25%2C+not+dead%2C+not+op_mini+all and https://browsersl.ist/?q=%3E+0.2%25%2C+not+dead%2C+not+op_mini+all%2C+not+ie+11%2C+not+safari+5.1 to see the browsers you'd be dropping support for (next change 91.67% to 89.43% - net change -2.24%).

Note that Polaris follows Shopify's browser support matrix which says the browsers excluded there aren't supported by us anyway. https://github.com/Shopify/polaris-react/blob/master/package.json#L87-L95

[BPScott]

Do they not have any tests in place at all?

We have plenty of tests, just none for browsers that we don't support, as that is kinda the definition of unsupported :)

Unfortunately in this case the transforms that CRA applies to attempt backwards compatibility for browsers older than the ones we support are unable to gracefully handle the valid modern css of top:calc(var(--control-border-width)*-1);.

Hopefully I'll be able to file an upstream bug in the coming days, and then once that is fixed and brought into CRA everything should be compiled as expected if you do not wish to modify your browser support matrix.

[mbaumbach]

@BPScott Great debugging! While I think it's good to have this fixed upstream so people with a vanilla CRA are less likely to be snagged by it, I also think it's totally reasonable to publish the browserslist config that Polaris officially supports and make that a requirement to use Polaris. So perhaps documenting it in the installation docs would be sufficient to state Polaris' supported browsers, would reduce people getting bit by it, and save you from having to dig through compiled CSS. :)

Some basic Googling doesn't seem to indicate that there's a way to inherit or reuse a browserslist config, otherwise it would be awesome to be able to depend on whatever is currently in the package.json for Polaris.

I'm going to move our browserslist config to what Polaris has.

[BPScott]

I also think it's totally reasonable to publish the browserslist config that Polaris officially supports

Newer versions of browserslist do let you extend from external configs. You might like https://github.com/shopify/browserslist-config, which is Shopify's general browserlist (polaris itself copy+pastes this intsead of extending from it for some annoying legacy reasons that I believe are on the cusp of going away soon)

[mbaumbach]

I also think it's totally reasonable to publish the browserslist config that Polaris officially supports

Newer versions of browserslist do let you extend from external configs. You might like https://github.com/shopify/browserslist-config, which is Shopify's general browserlist (polaris itself copy+pastes this intsead of extending from it for some annoying legacy reasons that I believe are on the cusp of going away soon)

That looks like a private repo, but I'm sure it's great. ;) Copying/pasting it isn't too big of a deal at the moment. I don't expect it to change frequently and if we see a compile problem that'll be the first place I check!

[BPScott]

Ah that's a little annoying - the package is public but the repo is private. In that case you can read the docs on npm, which is certainly public: https://www.npmjs.com/package/@shopify/browserslist-config

[mbaumbach]

Thanks @BPScott , that's very helpful!

[BPScott]

So I've managed to isolate the problem and have proven that this is indeed the custom-properties plugin of postcss-preset-env (eject the app and add features: { 'custom-properties': false}, to the postcss-env webpack config and things compile).

The good news is that this seems to be ultimately fixed upstream in postcss-custom-properties v9.0.1 already (CRA is currently using v8.0.11), however that update has not made its way through the dependency chain. We're waiting on an update in postcss-preset-env to bump its dependency on postcss-custom-properties, and then for CRA to depend upon that new postcss-custom-properties version.

As an alternative workaround to the browserlist rejigging above, you can force a dependency on the newest version of postcss-custom-properties using yarn. Add the following to your package.json, then run yarn and everything should compile:

"resolutions": {
  "postcss-custom-properties": "9.0.2"
},

[mbaumbach]

For those looking to just depend on the Shopify browserslist mentioned by Ben, this works fine in the latest CRA (3.3.0):

yarn add @shopify/browserslist-config

Then modify your browserslist config in the package.json to:

{
    "browserslist": ["extends @shopify/browserslist-config"]
}

[BPScott]

The plot thickens:

Turns out it's not calc(var(/**/)) stuff in general that's the problem, it's that the parser doesn't like the very specific ordering of having a custom property, then an operator, then a negative number with no spaces between the operator and the negative number.

Here's some testcases.

/* Crashes - patient 0 */
.x1 { top: calc(var(--x)*-1); }

/* Crashes -  space between the var and the operator */
.x2 {top: calc(var(--x) *-1); }

/* Crashes - a different operator */
.x3 {top: calc(var(--x)+-1); }


/* All the below compile successfully */

/* A space between the * and the negative number */
.a {top: calc(var(--x)* -1); }

/* A positive number instead of negative */
.b {top: calc(var(--x)*1); }

/* A regular value instead of a custom property */
.c {top: calc(3px*-1);}

/* flip the order */
.d {top: calc(-1*var(--x));}

/* add parenthesis */
.e { top: calc(var(--x)*(-1)); }

The old point still stands - this is fixed in postcss-custom-properties 9.0.1 but in the mean time we'll update our code so it will compile in spite of this bug

[pklitscher]

For those looking to just depend on the Shopify browserslist mentioned by Ben, this works fine in the latest CRA (3.3.0):

yarn add @shopify/browserslist-config

Then modify your browserslist config in the package.json to:

{
    "browserslist": ["extends @shopify/browserslist-config"]
}

You legend @mbaumbach !

This got me up and running.