Phase 7.5: Final cleanup — the parts ui-components does not block - #3136
Draft
OpenStaxClaude wants to merge 1 commit into
Draft
Phase 7.5: Final cleanup — the parts ui-components does not block#3136OpenStaxClaude wants to merge 1 commit into
OpenStaxClaude wants to merge 1 commit into
Conversation
The dependency removals this ticket was written around are blocked: @openstax/ui-components declares styled-components as a peer dependency and require()s it at runtime, and rex-web renders Footer, NavBar and ConfirmationToast from it. So the package, its types and the jest serializer all have to stay until CORE-1777 ships. What is not blocked: - ScrollOffset.spec and ScrollLock.spec each monkey-patched createGlobalStyle to work around it breaking on unmount without a dom. Both components are plain CSS now, so the patches were dead code that also kept a styled-components require in src/. - The prerender's collector no longer needs the /macro entry, since rex-web has no styled() call sites for babel-plugin-styled-components to transform. Importing the package directly makes the babelMacros config in package.json provably inert, so it goes too. - A guard test replaces the manual grep CORE-2285 used, because the package stays installed and resolvable for as long as ui-components takes to migrate. Verified by mutation: planting an import fails it. Also restores the alphabetical dependency ordering that #3132 disturbed when it removed and then restored these entries. CORE-2286
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The final-cleanup ticket, minus the parts that
@openstax/ui-componentsblocks.Stacked on
phase-7.4f-replace-createglobalstyle(#3132) — base is that branch, notmain.Jira: CORE-2286
Verifying the blocker rather than inheriting the claim
and rex-web imports that library from three places in the app shell:
src/app/components/Footer/index.tsxsrc/app/components/NavBar/index.tsxsrc/app/content/components/ConfirmationToast.tsxSo this is not a stale
package.jsonentry.FooterandNavBarare on every page, which puts styled-components in the always-loaded chunk of the production bundle. Four of the ticket's tasks are unreachable from this repo:styled-components— peer dependency of a library we import and render@types/styled-components— the prerender'sServerStyleSheetimport needs types; v4 ships nonejest-styled-components—src/test/setup.tsandsrc/test/utils.tsload the serializer, and ui-components still renders styled components into our snapshotsServerStyleSheetcollector — Phase 7.4f: Replace createGlobalStyle with stylis directly #3132 covers why removing it ships unstyled navbar/footerPlus the two verification tasks that depend on them (bundle size, full-suite-after-removal).
One thing worth knowing separately: rex-web pins
styled-components: "^4"(resolves to 4.4.1) while ui-components dev-tests against^5.3.5. The"*"peer range accepts our v4, so ui-components ships built against 5 and runs against 4 in this app. Pre-existing and not this ticket's problem, but it is a real latent risk and an argument for finishing CORE-1777 rather than upgrading.Bundle size: measured, and it is not zero
The ticket asks for a bundle size reduction. This PR's reduction is 0 bytes, because nothing is removed from the bundle — and I would rather state that than quietly skip the task.
The useful number is what the blocker is costing, so I measured it: production build, then a second build with
styled-componentswebpack-aliased to an empty stub, diffing total gzipped JS. Both builds withDISABLE_NEW_JSX_TRANSFORM=trueandGENERATE_SOURCEMAPnormalised.Essentially all of it sits in
519.*.chunk.js, whichbuild/index.htmlreferences directly — so it is initially-loaded weight on every page, not a lazy chunk. That chunk alone accounts for 15,934 of the 16,088 gzipped bytes, andsource-map-explorerputs the whole styled-components dependency tree there (@emotion/is-prop-valid,@emotion/unitless,@emotion/memoize,stylis-rule-sheet,memoize-one, plus nestedreact-isandprop-types).So ~15.7 KB gzipped is what CORE-1777 will actually free up, and it is currently on the critical path of every page load. That is the number this ticket was asking for; it just cannot be collected from this repo.
Why the totals are concatenated before gzipping
Summing
gzip -9per file across all 521 emitted chunks gave a net increase of 4.9 KB, which is an artifact: per-file gzip overhead times 521 files, plus webpack redistributing modules between chunks once the dependency dropped out. Concatenating in a stable order and compressing once removes both, and it agrees with the per-chunk figure for519(15,934 B) to within 154 bytes. I am reporting the concatenated number because the per-file sum is measuring the file count, not the code.source-map-exploreralso surfaced something specific to #3132 that is worth a look:stylis is in the bundle twice, ~11.8 KB of it duplicated, because #3132 added stylis as a direct dependency while styled-components still bundles its own. That is the correct trade (#3132 explains why depending on the transitive copy would be fragile) and it resolves itself when styled-components leaves — but it means #3132 costs ~11.8 KB until CORE-1777 lands, rather than being size-neutral. Flagging it so the choice is explicit.
What this PR actually does
1. Two dead monkey-patches removed
ScrollOffset.spec.tsxandScrollLock.spec.tsxboth did this in their "without a dom" setup:Both components were migrated to plain CSS in earlier phases —
ScrollOffset.tsandScrollLock.tsximportclassnamesand their own.css, and neither mentions styled-components. So the workaround was patching a function neither component calls any more. Removing it also removes two of the three remaining styled-components references insrc/; both suites still pass (19 tests).2. The prerender stops using the
/macroentry, andbabelMacrosgoesimport { ServerStyleSheet, StyleSheetManager } from 'styled-components/macro'was the only/macroimport left in the repo, and it was the only consumer of:pure: truemakes babel-plugin-styled-components add/*#__PURE__*/to styled component factory calls. rex-web has no such call sites left, so the option had nothing to annotate. Both exports are on the package's main entry (typeof ServerStyleSheet === 'function', checked against the installed 4.4.1), and babel-plugin-macros rewritesstyled-components/macrotostyled-componentsanyway — so this is the same module either way. Importing it directly makes thebabelMacrosremoval provably inert instead of merely-probably inert.I left
'macros'insrc/babel-config.js. Nothing imports a macro now, so it is a no-op plugin; removing it is defensible cleanup but it is shared script plumbing and not what the ticket asked for. Happy to take it out if you would rather.3. A guard test in place of CORE-2285's manual grep
CORE-2285 verified "zero styled-components imports" by grepping once. That was fine as an acceptance check, but the package now stays installed and resolvable from every file in the repo for however long CORE-1777 takes — with nothing stopping a new import from appearing.
src/noStyledComponents.spec.tsscanssrc/andscript/and asserts the set of importers equals a one-entry allowlist (the prerender, with a comment saying why). When ui-components ships a clean release the allowlist goes empty and the file is deleted with the dependency.Why a test and not an eslint rule:
.eslintrchasignorePatterns: ["*.spec.*", "src/test", "*.js"]andlint:typescriptonly runseslint src, sono-restricted-importswould have missed both spec files above and the wholescript/directory — i.e. every import this ticket is actually about.Mutation-checked, not assumed. Planting
import styled from 'styled-components'insrc/app/guards.tsfails it with+ "src/app/guards.ts"; removing it passes again. It also initially caught itself, because the doc comment contained a literal example import — the comment is now written not to match, and that is noted in the file so nobody reintroduces it.4. Dependency ordering restored
#3132 removed
@types/styled-componentsandjest-styled-componentsand then restored them, which left both out of alphabetical order. Moved back.Testing
tsc --noEmit— clean. (The guard neededreduce/concatrather thanflatMap; this project's TS lib target predates ES2019.)noStyledComponents.spec.ts— passes, and fails on a planted import as described above.ScrollOffset.spec.tsx+ScrollLock.spec.tsx— 19/19.npm run build—Compiled successfully., no warnings.test:unit— 241 suites / 1940 tests / 233 snapshots, all passing, zero snapshot changes.What still needs a ticket
CORE-2286 stays open after this merges, or gets split. The remaining work is one small PR that can only be written after ui-components ships a styled-components-free release and rex-web bumps its pin off
openstax/ui-components#1.21.1:styled-components,@types/styled-components,jest-styled-componentsServerStyleSheet/StyleSheetManagercollectorsrc/noStyledComponents.spec.tsI have asked on the ticket whether you want that as a new sibling subtask or tracked here.
🤖 Generated with Claude Code