Skip to content

Commit

Permalink
Remove SASS from @shopify/polaris (#11677)
Browse files Browse the repository at this point in the history
Part of https://github.com/Shopify/polaris-internal/issues/1345

- [x] I've tophatted [the
storybook](https://5d559397bae39100201eedc1-bldfeadhvc.chromatic.com/)
- [x] I've tophatted this [on
staging](https://web.docs.shopify.io/docs/guides/staging#testing-your-code-in-staging)
`staging-sxpt`

---

## Reviewing notes

The majority of changes in this PR are file renames (`.scss` -> `.css` &
their imports), which makes it hard to review the changes :(

To view the config related changes, use this command:

```bash
git diff main..HEAD -- ':!polaris-react/src/*' ':!polaris-react/postcss-mixins/*' ':!*.scss'
```

To see what the diff of the final generated .css is, read on...

### Diff details

As well as Tophatting Storybook & web, another way to confirm nothing
has broken / changed is comparing the built `styles.css` file agains
[the current
version](https://unpkg.com/browse/@shopify/polaris/build/esm/styles.css).

The output is not identical, but we didn't expect it to be (eg; moving
to CSS Custom Properties). We developed [a
script](https://github.com/Shopify/polaris/blob/remove-sass-compiled-css/go.sh)
to normalize the output between the two built `.css` files (one from
`main`, the other from this `remove-sass` branch) which minimizes the
diff to what's actually changed.

For the full diff with inline comments on what's changed and why, see
https://github.com/Shopify/polaris/pull/11718/files

---------

Co-authored-by: Charles Lee <[email protected]>
  • Loading branch information
jesstelford and gwyneplaine authored Mar 14, 2024
1 parent f9d5f25 commit f6ba2b2
Show file tree
Hide file tree
Showing 375 changed files with 1,860 additions and 2,619 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-rocks-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Migrated @shopify/polaris from SASS to CSS using postcss plugins
5 changes: 5 additions & 0 deletions .changeset/sharp-rings-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/stylelint-polaris': minor
---

Add `--pg-` as a disallowed CSS Custom Property prefix (these are "global" Custom Properties used within @shopify/polaris-react).
19 changes: 19 additions & 0 deletions .stylelintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module.exports = {
rules: {
'no-unknown-animations': null,
'value-keyword-case': ['lower', {camelCaseSvgKeywords: true}],
'at-rule-no-unknown': null,
'scss/at-rule-no-unknown': true,
},
overrides: [
{
Expand All @@ -17,6 +19,23 @@ module.exports = {
'function-disallowed-list': null,
},
},
{
files: ['polaris-react/postcss-mixins/*.css'],
rules: {
'comment-word-disallowed-list': [
/.*/,
{
message:
'Comments in polaris-react/postcss-mixins/*.css cause postcss-mixins to error. To disable lint rules, add them to styleslintrc.js overrides.',
},
],
'scss/at-rule-no-unknown': null,
// Yells at us about at rules. Since it's not configurable (see
// https://github.com/Shopify/polaris/pull/11709), we have to disable
// the entire rule.
'polaris/coverage': null,
},
},
],
ignoreFiles: [
'**/.next/**/*.{css,scss}',
Expand Down
18 changes: 18 additions & 0 deletions polaris-react/config/postcss-plugins.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
const path = require('path');

const postcssShopify = require('@shopify/postcss-plugin');
const postcssImport = require('postcss-import');
const pxtorem = require('postcss-pxtorem');
const postcssCustomMedia = require('postcss-custom-media');
const postcssGlobalData = require('@csstools/postcss-global-data');
const postcssNesting = require('postcss-nesting');
const postcssMixins = require('postcss-mixins');
const postcssDiscardComments = require('postcss-discard-comments');

const mediaQueriesCssPath = path.resolve(
__dirname,
'../../node_modules/@shopify/polaris-tokens/dist/css/media-queries.css',
);

module.exports = [
postcssImport(),
postcssMixins({
mixinsDir: path.join(__dirname, '../postcss-mixins'),
}),
postcssNesting({
// The way native CSS nesting & SASS nesting behave with complex selectors
// differ; SASS expands out every selector into a comma separated list, but
// native CSS wraps the complex selectors in an `:is()` which can result in
// a different specificity. We favour the SASS convention here to ensure
// compatibility with our ported-in SASS styles.
// See: https://sass-lang.com/blog/sass-and-native-nesting/
noIsPseudoSelector: true,
}),
postcssGlobalData({
files: [mediaQueriesCssPath],
}),
Expand All @@ -21,4 +38,5 @@ module.exports = [
replace: true,
propList: ['*'],
}),
postcssDiscardComments(),
];
2 changes: 1 addition & 1 deletion polaris-react/config/rollup/namespaced-classname.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function generateScopedName({includeHash = false} = {}) {
// Vite's esbuild appends query params to files under certain circumstances,
// so we clean those off in order to get the actual file name.
const cleanedFilename = filename.replace(/\?.*$/, '');
const componentName = basename(cleanedFilename, '.module.scss');
const componentName = basename(cleanedFilename, '.module.css');
const nestedComponentMatch =
NESTED_COMPONENT_PATH_REGEX.exec(cleanedFilename);

Expand Down
44 changes: 14 additions & 30 deletions polaris-react/config/rollup/plugin-styles.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const path = require('path');

const {createFilter} = require('@rollup/pluginutils');
const nodeSass = require('node-sass');
const postcss = require('postcss');
const postcssScss = require('postcss-scss');
const cssModules = require('postcss-modules');

module.exports.styles = function styles({
Expand Down Expand Up @@ -31,8 +31,7 @@ module.exports.styles = function styles({
]);

let inputRoot;

const processedExt = '.css';
const processedExt = '.out.css';

const cssByFile = {};

Expand All @@ -52,22 +51,21 @@ module.exports.styles = function styles({
function transformEsNext(rollup, id, postCssOutput) {
const relativePath = `./${path.relative(
path.dirname(id),
id.replace(/(\.module)?\.scss$/, processedExt),
id.replace(/(\.module)?\.css$/, processedExt),
)}`;

rollup.emitFile({
type: 'asset',
fileName: id
.replace(`${inputRoot}/`, '')
.replace(/(\.module)?\.scss$/, processedExt),
.replace(/(\.module)?\.css$/, processedExt),
source: postCssOutput.css,
});

const properties = JSON.stringify(postCssOutput.tokens, null, 2);
// No need to specify no-treeshake here as .css files get treated as
// external and thus their imports will not be tree shaken away anyway
return {
code: `import './${relativePath}';\nexport default ${properties};`,
code: `import '${relativePath}';\nexport default ${properties};`,
};
}

Expand Down Expand Up @@ -130,38 +128,24 @@ module.exports.styles = function styles({
inputRoot = path.resolve(process.cwd(), path.dirname(input[0]));
},

// Treat CSS files as external - don't try and resolve them within Rollup
// This only gets triggered in esnext mode when we emit imports of css files
// // Treat CSS files as external - don't try and resolve them within Rollup
// // This only gets triggered in esnext mode when we emit imports of css files
resolveId(source, importer) {
if (source.endsWith(processedExt)) {
return {
id: path.resolve(path.dirname(importer), source),
external: true,
};
}
if (!source.endsWith([processedExt])) return;
const id = path.resolve(path.dirname(importer), source);
return {
id,
external: true,
};
},

async transform(source, id) {
if (!filter(id)) {
return null;
}

let sassOutput;
try {
sassOutput = nodeSass
.renderSync({
data: source,
file: id,
outputStyle: 'compact',
includePaths: [path.dirname(id)],
})
.css.toString();
} catch (err) {
throw new Error(err.formatted);
}

const postCssOutput = await styleProcessor
.process(sassOutput, {from: id})
.process(source, {parser: postcssScss, from: id})
.then((result) => ({
css: result.css,
tokens: result.messages.find(({plugin, type}) => {
Expand Down
8 changes: 6 additions & 2 deletions polaris-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,16 @@
"chromatic": "^6.5.4",
"globby": "^11.1.0",
"js-yaml": "^4.1.0",
"node-sass": "^9.0.0",
"postcss": "^8.3.1",
"postcss-custom-media": "^10.0.1",
"postcss-discard-comments": "^6.0.1",
"postcss-import": "^15.1.0 ",
"postcss-loader": "^4.2.0",
"postcss-mixins": "^9.0.4",
"postcss-modules": "^4.2.2",
"postcss-pxtorem": "^5.1.1",
"postcss-nesting": "^12.0.2",
"postcss-pxtorem": "^6.0.0",
"postcss-scss": "^4.0.9",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"rimraf": "^3.0.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
@import '../src/styles/common';

.ContextControl {
display: flex;
align-items: center;
padding: 0 var(--p-space-300);
height: $top-bar-height;
// stylelint-disable-next-line -- polaris custom global property
height: var(--pg-top-bar-height);
}

.ShopName {
Expand Down
2 changes: 1 addition & 1 deletion polaris-react/playground/DetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import {
} from '../src';
import type {DropZoneProps, PageProps} from '../src';

import styles from './DetailsPage.module.scss';
import styles from './DetailsPage.module.css';

export function DetailsPage() {
const defaultState = useRef({
Expand Down
11 changes: 11 additions & 0 deletions polaris-react/postcss-mixins/base-button-disabled.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
@define-mixin base-button-disabled {
transition: none;
box-shadow: none;
border-color: var(--p-color-border-disabled);
background: var(--p-color-bg-fill-disabled);
color: var(--p-color-text-disabled);

svg {
fill: var(--p-color-icon-disabled);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
@mixin button-base {
$vertical-padding: calc(
(36px - var(--p-font-line-height-500) - var(--p-space-050)) / 2
);

// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($border-width: var(--p-border-width-025));
@define-mixin button-base {
@mixin focus-ring base, var(--p-border-width-025);
position: relative;
display: inline-flex;
align-items: center;
Expand Down Expand Up @@ -36,8 +31,7 @@

&:focus-visible {
box-shadow: var(--p-shadow-200);
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include no-focus-ring;
@mixin no-focus-ring;
outline: var(--p-border-width-050) solid var(--p-color-border-focus);
outline-offset: var(--p-space-025);
}
Expand Down Expand Up @@ -74,40 +68,3 @@
border: var(--p-border-width-025) solid windowText;
}
}

@mixin base-button-disabled {
transition: none;
box-shadow: none;
border-color: var(--p-color-border-disabled);
background: var(--p-color-bg-fill-disabled);
color: var(--p-color-text-disabled);

svg {
fill: var(--p-color-icon-disabled);
}
}

// Background color has been removed as a fix until plain buttons are revisited.
// As a result, this mixin doesn't visually add a backdrop anymore.
@mixin plain-button-backdrop {
padding: var(--p-space-050) var(--p-space-100);
margin: calc(-1 * var(--p-space-050)) calc(-1 * var(--p-space-100));
background: transparent;
border-radius: var(--p-border-radius-100);
}

@mixin unstyled-button() {
appearance: none;
margin: 0;
padding: 0;
background: none;
border: none;
font-size: inherit;
line-height: inherit;
color: inherit;
cursor: pointer;

&:focus {
outline: none;
}
}
61 changes: 61 additions & 0 deletions polaris-react/postcss-mixins/control-backdrop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
const {nullish} = require('./utils');

const DEFAULT_STYLE = 'base';

module.exports = (_, _style) => {
const style = nullish(_style) ? DEFAULT_STYLE : _style;
switch (style) {
case 'base': {
return {
position: 'relative;',
border: 'var(--p-border-width-050) solid var(--p-color-input-border);',
backgroundColor: 'var(--p-color-bg-surface);',
borderRadius: 'var(--p-border-radius-100);',
'&.hover,&:hover': {
cursor: 'pointer;',
borderColor: 'var(--p-color-border-hover);',
},
};
}
case 'active': {
return {
borderColor: 'var(--p-color-border-emphasis);',

'&::before': {
opacity: 1,
transform: 'scale(1);',
'@media (-ms-high-contrast: active)': {
border: 'var(--p-border-width-050) solid windowText;',
},
},
};
}
case 'disabled': {
return {
borderColor: 'var(--p-color-border-disabled);',

'&::before': {
backgroundColor: 'var(--p-color-bg-surface-disabled);',
},

'&:hover': {
cursor: 'default;',
},
};
}
case 'error': {
return {
borderColor: 'var(--p-color-border-critical);',
backgroundColor: 'var(--p-color-bg-fill-critical-secondary);',

'&.hover, &:hover': {
borderColor: 'var(--p-color-border-critical);',
},

'&::before': {
backgroundColor: 'var(--p-color-border-critical);',
},
};
}
}
};
10 changes: 10 additions & 0 deletions polaris-react/postcss-mixins/duplicate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = (_, ...selectors) => {
// Duplicate the styles across each of the given selectors
return selectors.reduce((result, selector) => {
// Strip any wrapping quotes for complex selectors
result[selector.replace(/^['"]?(.*?)['"]?$/, '$1')] = {
'@mixin-content': {},
};
return result;
}, {});
};
Loading

0 comments on commit f6ba2b2

Please sign in to comment.