feat(apollo-wind): default lucide icon stroke to 1.4 - #1085
Conversation
|
Apollo Coded App preview deployments are ready.
|
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
Pull request overview
Updates Apollo Wind’s default Lucide icon stroke weight to 1.4 via a narrowly-scoped CSS rule (only affecting Lucide’s default stroke-width="2"), and moves lucide-react from a direct dependency to a peer dependency in both apollo-wind and apollo-react to ensure consumers share a single module instance (avoiding duplicated types/instances).
Changes:
- Add a base-layer CSS rule to default Lucide’s
stroke-widthfrom2→1.4, while preserving non-defaultstrokeWidthvalues. - Safelist the
stroke-2utility so consumers of the prebuilt CSS have a documented opt-out. - Move
lucide-reacttopeerDependencies(kept asdevDependencyfor local build/test) inapollo-windandapollo-react, with corresponding lockfile updates.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Reflects dependency graph changes from moving lucide-react to peers/dev deps and related resolution updates. |
| packages/apollo-wind/src/templates/Patterns/ideas-AnimatedGradientText.tsx | Removes a redundant explicit strokeWidth={2} so the new default styling applies. |
| packages/apollo-wind/src/styles/tailwind.consumer.css | Adds the svg.lucide[stroke-width="2"] { stroke-width: 1.4; } rule and safelists stroke-2 for the prebuilt CSS output. |
| packages/apollo-wind/package.json | Moves lucide-react to peerDependencies (and keeps it in devDependencies for build/test). |
| packages/apollo-react/package.json | Moves lucide-react to peerDependencies (and keeps it in devDependencies for build/test). |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
f78dac5 to
71d8ace
Compare
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Aug 25, 2026, 06:03:56 PM |
📊 Coverage + size by packagePer-package bundle size on this PR (no JS/TS source changes detected under
"Coverage" is each package's own |
lucide's 2px stroke reads too heavy against Apollo's type; design confirmed 1.4
system-wide.
lucide has no theming API, so CSS is the only lever, and a stylesheet outranks
the presentation attribute the prop compiles to (cascade layers included). The
rule therefore matches lucide's default value only, leaving deliberate weights
like strokeWidth={0} intact. Only 2 is ambiguous, so `stroke-2` is the
documented opt-out, safelisted so it exists in the prebuilt styles.css too.
Also moves lucide-react to peerDependencies in apollo-wind and apollo-react so
consumers share one copy — LucideIcon is already public API, so passing your own
icon means passing a type from a different module instance.
Refs: MST-14019
71d8ace to
524c08c
Compare
github-actions
Bot
added
size:L
BenGSchulz
added
the
dev-packages
📦 Dev Packages
|
Sets lucide's icon stroke to 1.4 for every consumer of the apollo-wind styles, and moves
lucide-reactto a peer dependency so consumers share a single copy.Design confirmed 1.4 as the system-wide default. Filed from the Flow UX Audit (MST-14019), but the fix belongs here rather than in a consumer: apollo-wind renders its own lucide icons (accordion chevrons, select carets), which a downstream rule can't reach cleanly.
Why CSS, and why the selector looks like that
On the version range we support today, lucide has no theming API —
Iconhard-codesstrokeWidth = 2as a default parameter and spreads it onto the SVG. So CSS is the only lever. (1.x changes this — see the last section.)The catch is that a stylesheet outranks the presentation attribute the prop compiles to, so a blanket
svg.lucide { stroke-width: 1.4 }would silently break everystrokeWidthprop in every consumer. I verified this in Chromium rather than trusting the spec — cascade layers do not change it; a layered rule still beatstroke-width="3"andstroke-width="0".So the rule matches lucide's default value only:
strokeWidth={0}(filled glyphs)strokeWidth={3}className="stroke-2"Only the value
2is indistinguishable from "no opinion", so astrokeWidth={2}meaning keep this bold is the one case that would be silently thinned.stroke-2is the documented opt-out — it lands in theutilitieslayer, which the cascade resolves afterbaseregardless of specificity, so it wins.@source inline("stroke-2")safelists it. Tailwind only emits utilities it finds while scanning source, and nothing in apollo-wind writesstroke-2— without the safelist the escape hatch would be missing from the prebuiltstyles.cssfor consumers who ship that file instead of building fromtailwind.css. Verified against the real built artifact, both before and after.lucide-react as a peer dependency
lucide-reactwas a harddependencyof apollo-wind and apollo-react, so consumers ended up with a second copy. It's also already part of the public type surface —NodeOutputModeIconis aLucideIcon, as isLockableValueField'siconprop — so a consumer passing its own icon is passing a type from a different module instance.Range is
>=0.577.0 <2— deliberately spanning both current majors, since everything apollo uses (Icon,createLucideIcon,LucideIcon,LucideProps, the named icons) is present in both. Both packages keep it as adevDependency. Released as a minor, not a major: the range is permissive and auto-install-peers is the default in npm 7+, pnpm 8+, and yarn berry.Follow-up:
LucideProvidershould eventually replace this ruleWorth recording, since it changes how the CSS above should be maintained rather than merely extended.
lucide 1.x (latest is 1.34.0; we're on 0.577.0) adds
LucideProvider, andIconnow resolvesstrokeWidth ?? contextStrokeWidth. Nullish coalescing meansstrokeWidth={0}survives andstrokeWidth={2}is genuinely honored — it makes 1.4 a real React default and removes the ambiguity this PR works around, with no selector and no safelist.Important: the two mechanisms don't compose. Under a provider a deliberate
strokeWidth={2}renders as2and this rule would thin it all over again. Replace the rule with the provider; don't run both. The CSS comment says the same thing in-place.Why this isn't bundled here
Adopting the provider means a peer floor of
>=1, which is a genuinely breaking change for every consumer — and a provider only covers the React tree it wraps. apollo-wind has no root provider to host it, so each consuming app would mount its own, and consumers with multiple React roots (flow-workbench's<ui-flow-canvas>web components, Studio Web's module federation) would need per-root coverage verified. The CSS rule needs zero consumer code and works on 0.x and 1.x alike.What a
>=1floor would actually forceThe icon set loses 29 keys from the runtime
iconsmap (1703 → 1777; +103, −29). They split into three tiers with very different consequences:import { X }icons['X']lookupThe renamed 11:
History→RotateCcwClock,Waves→WavesHorizontal,Smile→FaceSlightlySmiling,Frown→FaceSlightlyFrowning,Meh→FaceNeutral,Angry→FaceAngry,Annoyed→FaceExpressionless,Laugh→FaceGrinning,SmilePlus→FaceSlightlySmilingPlus,Podcast→MicSignal,TextSelect→SquareDashedText.The compile-time surface is small — 18 brand icons, and neither apollo-ui nor flow-workbench imports any of them.
LucideIcon/LucidePropsare intact, aliases likeAlertTriangle → TriangleAlertare preserved, and 1.x still emitsclass="lucide", so this PR's rule stays valid either way.The hazard is the data-driven path. The
iconsmap holds canonical names only, soicons['History']is nowundefinedfor all 29.apollo-react/src/canvas/utils/icon-registry.tsxdoes exactly that lookup and falls back toBox— no error, no crash, no log. flow-workbench feeds itdisplay.icon, typedz.string().optional()and resolved from server-fetched connector manifests. A Slack or GitHub connector icon would silently render as a generic box in production, and because the data isn't in the repo, no grep can clear it.Cheap mitigation, worth landing ahead of any upgrade: a ~30-line alias map in
icon-registry.tsxtranslating the 29 legacy ids to their new canonical names before theBoxfallback. That turns the silent tier into a non-event and is harmless on 0.x.Packaging changes in 1.x
dist/esm/lucide-react.js→.mjs. Neither repo deep-imports, so it's a non-issue here; it breaks anyone who does.unpkgandmain:umdremoved — no UMD/CDN build."use client"added toIconandcontext. Benign for Vite/rsbuild; relevant for RSC consumers and may surface directive warnings in some Rollup setups.^16.5.1 || ^17 || ^18 || ^19).Net: the cost isn't "fix 18 imports", it's "audit runtime manifest data across every consumer, including connector icons we don't control" — a coordination problem with Studio Web and the connector teams, not an apollo-local change.
Scope notes for review
<Sparkles strokeWidth={2}>inideas-AnimatedGradientText.tsx. I swept all 43strokeWidth=2sites in the repo and classified each by enclosing tag; this is the only one on a lucide icon. The rest are recharts (Line/Pie/Radar), raw SVG primitives, and hand-authored icon components — all of which would drop to 1, not 1.4, if the prop were removed. They're untouched.token-icon-markup.tsthins, correctly — it hand-builds markup carryingclass="lucide"precisely to mirror lucide's output. ThestrokeWidth={2}intree-view.tsxandTokenPill.tsxare hand-rolled<svg><path>with nolucideclass and are unaffected.package.json, not inglobals.css— so it never receives this rule, andDESIGN-CONTEXT.mddocuments it as a distinct design system. Its icon catalogue hard-codes 1.5; whether Vertex tracks 1.4 is a Vertex design call, not this PR's.Testing
typecheck,build,lint,format:checkgreen; 1409 (wind) + 2585 (react) unit tests pass.New coverage in
src/styles/tailwind.consumer.test.ts, following the file's existing pattern of parsing the selector out of the stylesheet so the cases can't drift from what ships: the rule is brace-matched to be inside@layer base, the safelist is asserted, and the selector is exercised against real elements — a default lucide icon matches, whilestroke-width0 / 1 / 1.5 / 2.5 / 3 and non-lucide svgs (recharts series, edge handles, hand-authored icons) do not.Mutation-checked rather than assumed — each guard was confirmed to fail when the CSS is broken:
@source inline("stroke-2")svg.lucideselector@layer basetest:visualsnapshots will need a wholesale update — this moves nearly every icon in the system by design. I deliberately have not run or approved them; that's a reviewer/design judgement.