Generate blocking + non-blocking CSS files that can be shared across apps #636
Conversation
andygout
requested review from
i-like-robots,
apaleslimghost and
jenniferemshepherd
| 'shared-blocking-styles': '../../packages/dotcom-ui-layout/shared-blocking-styles.scss', | ||
| 'shared-non-blocking-styles': '../../packages/dotcom-ui-layout/shared-non-blocking-styles.scss' |
There was a problem hiding this comment.
this means that apps have to be aware of the implementation details of dotcom-ui-layout, and if we add any more packages with shared blocking/nonblocking styles it's quite cumbersome.
me and @i-like-robots discussed making this more abstract, potentially by having a page-kit-cli plugin, along with a separate package.json field, could emit a separate chunk for a dependency's nonblocking styles (this could also then feed into the manifest & asset loader)
There was a problem hiding this comment.
For now I think we should go with require.resolve('@financial-times/dotcom-ui-layout/shared-blocking-styles.scss') etc. to avoid the relative path shenanigans.
As Kara hints, we're probably going to massage this feature a few times yet until we get it right!
| @include oHeader($features: ('drawer', 'megamenu', 'sticky'), $include-base-styles: false); | ||
|
|
||
| // Megamenu z-indexes etc. | ||
| @import "@financial-times/dotcom-ui-header/styles"; |
There was a problem hiding this comment.
this is also including all of o-header, so that output is being duplicated
There was a problem hiding this comment.
I think this comes from my original example - sorry!
We'll probably need an async/blocking stylesheet for all Page Kit UI components to make this consistent and work 🤔
|
Closing this in favour of #665 as we have decided to tackle one thing at a time. We are currently not sure if it there is enough - or any - performance gain generating separate blocking/non-blocking CSS bundles for the shared styles so we will avoid this complexity until it becomes necessary. |
Addresses #546 (or at least is one approach to fixing one part of it… I feel this PR should probably not close the issue entirely).
Dependent on: #611.