Skip to content

Commit 71a508f

Browse files
arbrandesclaude
andcommitted
fix: cascade-layer order
CSS cascade-layer priority is locked in by the first mention of each layer name. With the order statement living at the top of `shell/style.scss`, other shell SCSS files reached earlier by webpack's import traversal were registering `shell` before `paragon` and inverting the intended order. Move the declaration to its own `shell/layer-order.scss`, loaded first via a dedicated webpack entry. Also classify named packages by package.json name, where possible. This makes it more flexible (so that it applies to workspaces, for instance). Fixes #246. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 08f1cc6 commit 71a508f

10 files changed

Lines changed: 70 additions & 16 deletions

docs/decisions/0008-stylesheet-import-in-site-config.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ The build pipeline wraps each stylesheet in a CSS cascade layer based on the res
3434
| `site` | stylesheets outside `node_modules` (the composing site's source) |
3535
| `brand` | `@(open)?edx/brand*` packages |
3636

37-
The order declared at the top of `shell/style.scss` is `@layer paragon, shell, app, site, brand;` so the cascade resolves in that order: `brand` wins over `site`, `site` wins over `app`, `app` wins over `shell`, and `shell` wins over `paragon`.
37+
The order is declared in `shell/layer-order.scss` as `@layer paragon, shell, app, site, brand;`, so the cascade resolves in that order: `brand` wins over `site`, `site` wins over `app`, `app` wins over `shell`, and `shell` wins over `paragon`.
3838

3939
`brand` is last because, in production, brand CSS is injected at runtime via `<link>` tags that bypass webpack entirely and therefore land **unlayered**. Unlayered rules beat every layered rule regardless of declared order, so runtime brand wins over the site's own CSS. Putting build-time brand imports (e.g. a dev harness that `@use`s a brand package directly) in the last layer keeps dev harness behavior consistent with production: brand overrides apply on top of everything the site declares.
4040

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
".": "./dist/index.js",
1010
"./tools": "./dist/tools/index.js",
1111
"./tools/tsconfig.json": "./dist/tools/typescript/tsconfig.json",
12+
"./shell/layer-order.scss": "./dist/shell/layer-order.scss",
1213
"./shell/style": "./dist/shell/style.js",
1314
"./shell/site": "./dist/shell/site.js"
1415
},

shell/layer-order.scss

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@layer paragon, shell, app, site, brand;

shell/style.scss

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
@layer paragon, shell, app, site, brand;
2-
31
.flex-basis-0 {
42
flex-basis: 0 !important;
53
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import {
2+
APP_RESOURCE,
3+
BRAND_PACKAGE,
4+
} from './getStylesheetRule';
5+
6+
describe('stylesheet classification', () => {
7+
describe('BRAND_PACKAGE', () => {
8+
const re = BRAND_PACKAGE.name as RegExp;
9+
10+
it('matches @openedx/brand-*', () => {
11+
expect(re.test('@openedx/brand-openedx')).toBe(true);
12+
});
13+
14+
it('matches @edx/brand-*', () => {
15+
expect(re.test('@edx/brand-foo')).toBe(true);
16+
});
17+
18+
it('matches bare @openedx/brand', () => {
19+
expect(re.test('@openedx/brand')).toBe(true);
20+
});
21+
22+
it('does not match unrelated scoped packages', () => {
23+
expect(re.test('@openedx/paragon')).toBe(false);
24+
expect(re.test('@openedx/branding')).toBe(false);
25+
});
26+
});
27+
28+
describe('APP_RESOURCE', () => {
29+
it('matches a node_modules path', () => {
30+
expect(APP_RESOURCE.test('/site/node_modules/some-pkg/index.js')).toBe(true);
31+
});
32+
33+
it('matches an npm-workspace packages path', () => {
34+
expect(APP_RESOURCE.test('/site/packages/some-pkg/index.js')).toBe(true);
35+
});
36+
37+
it('does not match a site source path', () => {
38+
expect(APP_RESOURCE.test('/site/src/component.scss')).toBe(false);
39+
});
40+
});
41+
});

tools/webpack/common-config/all/getStylesheetRule.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ import postcssWrapLayer from './postcssWrapLayer';
2121
* precedence of runtime brand CSS (which is injected unlayered via
2222
* <link> tags and thus beats every layered rule).
2323
*/
24-
const PARAGON_RESOURCE = /@openedx[\\/]paragon[\\/]/;
25-
const SHELL_RESOURCE = /(@openedx[\\/]frontend-base|frontend-base[\\/]shell)[\\/]/;
26-
const BRAND_RESOURCE = /@(open)?edx[\\/]brand(-[^\\/]+)?[\\/]/;
27-
const NODE_MODULES = /[\\/]node_modules[\\/]/;
24+
export const PARAGON_PACKAGE = { name: '@openedx/paragon' };
25+
export const SHELL_PACKAGE = { name: '@openedx/frontend-base' };
26+
export const BRAND_PACKAGE = { name: /^@(open)?edx\/brand(-.+)?$/ };
27+
// The site/app split is positional (my project vs. a dep), so it is path-based.
28+
export const APP_RESOURCE = /[\\/](node_modules|packages)[\\/]/;
2829

2930
/*
3031
* There are a few things we need to do here.
@@ -40,7 +41,7 @@ export default function getStylesheetRule(mode: 'dev' | 'production'): RuleSetRu
4041
test: /(.scss|.css)$/,
4142
oneOf: [
4243
{
43-
resource: PARAGON_RESOURCE,
44+
descriptionData: PARAGON_PACKAGE,
4445
// We need Paragon to not elide CSS: we have to be able to import it
4546
// directly from shell/style.ts
4647
sideEffects: true,
@@ -50,21 +51,21 @@ export default function getStylesheetRule(mode: 'dev' | 'production'): RuleSetRu
5051
],
5152
},
5253
{
53-
resource: SHELL_RESOURCE,
54+
descriptionData: SHELL_PACKAGE,
5455
use: [
5556
MiniCssExtractPlugin.loader,
5657
...getStyleUseConfig(mode, 'shell'),
5758
],
5859
},
5960
{
60-
resource: BRAND_RESOURCE,
61+
descriptionData: BRAND_PACKAGE,
6162
use: [
6263
MiniCssExtractPlugin.loader,
6364
...getStyleUseConfig(mode, 'brand'),
6465
],
6566
},
6667
{
67-
resource: { not: [NODE_MODULES] },
68+
resource: { not: [APP_RESOURCE] },
6869
use: [
6970
getFirstLoader(mode),
7071
...getStyleUseConfig(mode, 'site'),
Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
import HtmlWebpackPlugin from 'html-webpack-plugin';
22
import path from 'path';
33

4-
// Generates an HTML file in the output directory.
4+
/*
5+
* Generates an HTML file in the output directory.
6+
*
7+
* `layerOrder` is listed first so its <link> is injected before any other
8+
* stylesheet. That file contains the `@layer paragon, shell, app, site, brand;`
9+
* statement, which locks in cascade-layer priority before any layer name is
10+
* mentioned by the rest of the CSS the browser parses.
11+
*/
512
export default function getHtmlWebpackPlugin() {
613
return new HtmlWebpackPlugin({
7-
inject: true, // Appends script tags linking to the webpack bundles at the end of the body
14+
inject: true,
815
template: path.resolve(process.cwd(), 'public/index.html'),
9-
chunks: ['app'],
16+
chunks: ['layerOrder', 'app'],
17+
chunksSortMode: 'manual',
1018
});
1119
}

tools/webpack/webpack.config.build.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const config: Configuration = {
2626
mode: 'production',
2727
devtool: 'source-map',
2828
entry: {
29+
layerOrder: '@openedx/frontend-base/shell/layer-order.scss',
2930
app: '@openedx/frontend-base/shell/site',
3031
},
3132
output: {

tools/webpack/webpack.config.dev.shell.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const resolvedSiteI18nPath = getResolvedSiteI18nPath('test-site/src/i18n');
2424

2525
const config: Configuration = {
2626
entry: {
27+
layerOrder: path.resolve(process.cwd(), 'shell/layer-order.scss'),
2728
app: path.resolve(process.cwd(), 'shell/site'),
2829
},
2930
output: {
@@ -90,9 +91,10 @@ const config: Configuration = {
9091
filename: '[name].css',
9192
}),
9293
new HtmlWebpackPlugin({
93-
inject: true, // Appends script tags linking to the webpack bundles at the end of the body
94+
inject: true,
9495
template: path.resolve(process.cwd(), 'shell/public/index.html'),
95-
chunks: ['app'],
96+
chunks: ['layerOrder', 'app'],
97+
chunksSortMode: 'manual',
9698
}),
9799
new ReactRefreshWebpackPlugin(),
98100
new ForkTsCheckerWebpackPlugin(),

tools/webpack/webpack.config.dev.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const resolvedSiteI18nPath = getResolvedSiteI18nPath('src/i18n');
2525

2626
const config: Configuration = {
2727
entry: {
28+
layerOrder: '@openedx/frontend-base/shell/layer-order.scss',
2829
app: '@openedx/frontend-base/shell/site',
2930
},
3031
output: {

0 commit comments

Comments
 (0)