-
Notifications
You must be signed in to change notification settings - Fork 28
feat(devBundler): aggregate styles into dependencies and local overrides #557
Conversation
packages/one-app-dev-bundler/esbuild/utils/server-style-aggregator.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Matthew Mallimo <[email protected]>
packages/one-app-dev-bundler/esbuild/utils/server-style-aggregator.js
Outdated
Show resolved
Hide resolved
packages/one-app-dev-bundler/esbuild/utils/server-style-aggregator.js
Outdated
Show resolved
Hide resolved
This feature needs to have parity across development and production environments, as this bundler should only be used in development deduplication needs to also be possible using one-app-bundler. |
100% I wanted to raise this as an initial proof of concept that everybody was happy with the structure and approach before replicating the work into the prod bundler Would we rather that it were 2 PRs to different packages, or 1 PR for the feature? |
7a26f65
to
c57043b
Compare
It's a cool feature, up to you on how you do it. I think as long as its noted that PR is open(could be a draft) to the other bundler i don't think it needs to block this PR |
Provide a general summary of your changes in the Title above.
Description
Describe your changes in detail
This PR achieves two primary features:
Coupled with an adjustment to the One App style loader, this will allow styles to be rendered into the page consistently between Client and Server renders, remove an unnecessary style recalculation step from the browser, and remove duplicated styles being rendered onto the page for each module.
Motivation
Why is this change required? What issue does it resolve?
Currently when bundling a One App module, the styles are added to the page inconsistently between the server and client side bundles. The server bundle adds the styles "upside down" with local application styles appearing first, and dependency styles appearing last.
In addition, once the client bundle has hydrated the styles to the expected order, duplicate style tags are rendered multiple times, resulting in local application styles being ignored.
Test Conditions
Describe in detail how the changes are tested.
Include details of your testing environment, and the tests you ran to.
How does your change affect the rest of the code.
Tested running a root module with three child modules each serving a shared dependency component library. Each module renders a component twice. Once with no changes, and once with style overrides local to the module.
The expected output is three modules, showing a green component, and then a blue/red component with text that has an orange background color.
The expected DOM output was three style tags, one with the styles for the shared component (deduplicated twice), one with the styles for the blue override (deduplicated once between Module One and Two), and one with the styles for the red override.
For backwards compatibility, a getFullSheet function is included that returns a single string of all the aggregated styles as a string. This allows for running an updated bundle in a version of OneApp that doesn't support the style aggregation.
To note - this getFullSheet does correctly organise the styles to put dependencies before local overrides, which would partially solve the problem where styles are inconsistent between SSR and CSR, so even in an older version of One App, a modern bundle would result in more consistency when hydrated.
Types of changes
Check boxes that apply:
Checklist
Check boxes that apply: