Follow-up to #505. only-include-used-components relies on two hand-written maps in src/bin/only-include-used-components.ts, and neither has a guard. Both failure modes are silent: the script still exits 0.
1. REACT_DSFR_MODULE_TO_DSFR_COMPONENTS has no exhaustiveness check
A module absent from it and from NON_COMPONENT_MODULE_IDS makes resolveModuleIdToDsfrComponents return undefined, which trips the include-everything fail-safe. Adding a component to src/ is enough to disable the optimization for every consumer importing it.
Reproduced on #512: resolveModuleIdToDsfrComponents({ moduleId: "ContentMedia" }) returns undefined. Same result for the directory form (src/ContentMedia/index.tsx) and for a new src/blocks/* entry.
The maintainer merging such a PR gets no signal, and no existing test fails.
2. content is detected on core utility classes, causing over-inclusion
DSFR_COMPONENT_DETECTION_CLASS_PREFIXES["content"] lists fr-responsive-img and fr-responsive-vid. Those are core classes, not content classes:
.fr-responsive-img and its 7 ratio modifiers are defined in core.main.css, which is always included.
- All 8 occurrences in
content.main.css are scoped under .fr-content-media, 0 unscoped.
So any project using fr-responsive-img pulls in the whole content component for nothing. Measured end to end on a consumer using fr-responsive-img without fr-content-media:
|
components |
dsfr.min.css |
| current |
3 / 45 |
234 877 B |
| without the two prefixes |
2 / 45 |
231 924 B |
Dropping them is safe: the only content rules involving those classes require .fr-content-media, which is already a detection prefix on its own.
Proposal
Two tests that re-derive each map from a machine source rather than restating it, plus the one-line fix for 2:
- Enumerate the public subpaths from the real
src/ layout (minus what src/tsconfig.json excludes), run them through the production module id extraction, assert each resolves.
- Assert every detection prefix opens a selector in its own component stylesheet, read from the installed
@gouvfr/dsfr. Merely appearing in it is not enough, which is exactly what 2 gets wrong.
Both should be checked against the case that makes them answer false, not only the nominal one.
I have this ready and will open a PR.
Follow-up to #505.
only-include-used-componentsrelies on two hand-written maps insrc/bin/only-include-used-components.ts, and neither has a guard. Both failure modes are silent: the script still exits 0.1.
REACT_DSFR_MODULE_TO_DSFR_COMPONENTShas no exhaustiveness checkA module absent from it and from
NON_COMPONENT_MODULE_IDSmakesresolveModuleIdToDsfrComponentsreturnundefined, which trips the include-everything fail-safe. Adding a component tosrc/is enough to disable the optimization for every consumer importing it.Reproduced on #512:
resolveModuleIdToDsfrComponents({ moduleId: "ContentMedia" })returnsundefined. Same result for the directory form (src/ContentMedia/index.tsx) and for a newsrc/blocks/*entry.The maintainer merging such a PR gets no signal, and no existing test fails.
2.
contentis detected on core utility classes, causing over-inclusionDSFR_COMPONENT_DETECTION_CLASS_PREFIXES["content"]listsfr-responsive-imgandfr-responsive-vid. Those are core classes, not content classes:.fr-responsive-imgand its 7 ratio modifiers are defined incore.main.css, which is always included.content.main.cssare scoped under.fr-content-media, 0 unscoped.So any project using
fr-responsive-imgpulls in the wholecontentcomponent for nothing. Measured end to end on a consumer usingfr-responsive-imgwithoutfr-content-media:Dropping them is safe: the only content rules involving those classes require
.fr-content-media, which is already a detection prefix on its own.Proposal
Two tests that re-derive each map from a machine source rather than restating it, plus the one-line fix for 2:
src/layout (minus whatsrc/tsconfig.jsonexcludes), run them through the production module id extraction, assert each resolves.@gouvfr/dsfr. Merely appearing in it is not enough, which is exactly what 2 gets wrong.Both should be checked against the case that makes them answer false, not only the nominal one.
I have this ready and will open a PR.