@@ -13,77 +13,97 @@ a version bump in the **Governance** section below.
1313### 1. Backward-compatible public API
1414
1515The contract exposed in [ PickupPointsModal.js] ( ../../react/PickupPointsModal.js )
16- is consumed by production workspaces across hundreds of VTEX stores . Any
16+ is consumed by production workspaces of stores using VTEX Checkout . Any
1717change to props, events, or the shape of ` orderForm ` /` logisticsInfo `
1818required by the component is ** breaking** and demands a major bump in
19- ` manifest.json ` .
19+ ` manifest.json ` (currently at ` 3.x ` ) .
2020
21- - ** Why:** VTEX IO apps receive this package via ` 1.x ` / ` 3.x ` dependency
22- ranges; silently breaking the contract takes down checkout in
23- production stores.
21+ - ** Why:** VTEX IO apps and the NPM package ` @vtex/pickup-points-modal `
22+ receive this code via ` 1.x ` / ` 3.x ` dependency ranges; silently breaking
23+ the contract takes down checkout in production stores.
2424- ** How to apply:** any PR touching ` PickupPointsModal.js ` requires a
2525 dedicated review focused on the public contract plus an explicit
2626 CHANGELOG entry.
2727
28- ### 2. Tested behavior over coverage number
28+ ### 2. Tested behavior, not coverage number
2929
30- Every new piece of logic requires Jest tests covering:
31-
32- - The expected case (golden path)
33- - At least one error / edge case
34- - No visible regression: tests for the feature being modified continue
35- to pass
36-
37- There is no absolute coverage target — what matters is that every
38- behavioral change is anchored to a test.
30+ Behavioral changes require Jest tests that exercise the new behavior. The
31+ project does not target a fixed coverage percentage — what matters is
32+ that each shipped change has at least one test pinning the intended
33+ behavior.
3934
4035- ** Why:** the codebase mixes legacy JS and TS, and VTEX's ` orderForm `
41- has a shape with many optional fields. Coverage-by-number lies — bugs
42- show up in combinations that were not in the plan.
43- - ** How to apply:** PRs with behavioral changes but no new test go back
44- to the author. Bugs detected in production must be reproduced with a
45- test ** before** the fix.
36+ has many optional fields. Coverage-by-number is misleading — bugs show
37+ up in combinations that were not in the plan. Today's test suite is
38+ intentionally narrow (~ 11 test files); this principle keeps the bar
39+ on * new* behavior rather than retroactively asking for full coverage.
40+ - ** How to apply:** PRs that change behavior without adding or updating
41+ a test are pushed back to the author. Reproducing a production bug
42+ with a failing test before the fix is the strongly preferred default,
43+ not a hard gate.
4644
4745### 3. Internationalization is non-negotiable
4846
4947Every user-facing string goes through ` react-intl ` . New keys must be
50- added to ** all** locales present in ` messages/ ` in the same PR — even if
51- the value is the original English string as a temporary fallback.
48+ added to ** all** locale files present in ` messages/ ` in the same PR —
49+ even if the value is the original English string as a temporary
50+ fallback. ` messages/context.json ` is the English master and the source
51+ of truth for keys.
52+
53+ The component currently ships with 31 locale files in ` messages/ `
54+ (plus ` context.json ` as the English master):
55+
56+ ```
57+ ar-SA bg ca cs da de el en
58+ en-GB en-LB en-MM en-US es fi fr id-ID
59+ it ja ko nl nn no pl pt-BR
60+ pt-PT ro ru sk sl sv uk
61+ ```
5262
53- - ** Why:** the component runs across BR/EN/ES/MX/PT stores in
54- production; missing a key in one locale renders ` Missing translation `
55- visibly in checkout.
56- - ** How to apply:** run ` yarn lint:locales ` before opening the PR.
57- ` intl-equalizer ` must pass.
63+ - ** Why:** the component runs across all the locales above in
64+ production; missing a key in any one of them renders
65+ ` Missing translation ` visibly in checkout.
66+ - ** How to apply:** run ` yarn lint:locales ` ( ` intl-equalizer ` ) before
67+ opening the PR. It must pass.
5868
59- ### 4. Performance budget on the checkout path
69+ ### 4. Performance on the checkout-open path
6070
6171The modal opens in the critical purchase-completion flow. Changes must
6272not:
6373
64- - Add network requests in the initial ` componentDidMount ` /` useEffect `
65- without cache or debounce.
66- - Load heavy libraries (>50KB minified) into the main bundle — use
67- dynamic import if strictly necessary.
68- - Break the lazy-load of Google Maps (currently loaded on demand).
69-
70- - ** Why:** checkout abandonment rises ~ 1% for every additional 100ms of
71- TTI according to internal Checkout team analyses.
72- - ** How to apply:** PRs that add a dependency must justify size in the
73- description. Suspected performance regressions open a profiling task
74- before merge.
75-
76- ### 5. Side-effect free utilities
77-
78- Functions in ` react/utils/ ` and ` react/fetchers/ ` must be pure or
79- declare the side effect in the name (` fetchX ` , ` saveY ` ). No singletons,
80- no hidden global state.
81-
82- - ** Why:** makes it easy to unit-test without mounting the full
83- component tree with providers.
84- - ** How to apply:** PRs that add global state to a utility go back to
85- the author with a suggestion to move it into ` containers/ ` or
86- ` ModalState.js ` .
74+ - Add unconditional network requests on initial render
75+ (` componentDidMount ` /` useEffect ` ) without cache or debounce.
76+ - Pull heavy libraries into the main bundle when a dynamic import or a
77+ smaller alternative would do the job.
78+ - Break the existing gated load of Google Maps in
79+ [ ` containers/withGoogleMaps.js ` ] ( ../../react/containers/withGoogleMaps.js )
80+ (the map only initializes when ` googleMapsKey ` is provided, via
81+ ` GoogleMapsContainer ` from ` vtex.address-form ` ).
82+
83+ - ** Why:** performance regressions on this path are visible to every
84+ user who reaches the shipping step. There is no published numeric
85+ budget — judgment is required, and PRs that grow bundle size or add
86+ blocking work must justify the cost in the description.
87+ - ** How to apply:** PRs that add a dependency must explain the size and
88+ loading strategy in the description. Suspected regressions open a
89+ profiling task before merge.
90+
91+ ### 5. Side-effect aware utilities
92+
93+ Functions under ` react/utils/ ` should be pure. Functions that perform
94+ I/O or telemetry must declare it in the name (` fetchX ` , ` saveY ` , the
95+ ` ...Event ` suffix used in [ ` utils/metrics.js ` ] ( ../../react/utils/metrics.js ) ).
96+ Network calls live under ` react/fetchers/ ` .
97+
98+ - ** Why:** this makes it easy to unit-test utilities without mounting
99+ the full component tree with providers.
100+ - ** How to apply:** PRs that add global state to a utility (a singleton,
101+ a mutable module-level cache) go back to the author with a suggestion
102+ to move it into ` containers/ ` or ` ModalState.js ` .
103+ - ** Known exception:** ` react/fetchers/index.js ` installs a global
104+ ` axios-retry ` interceptor at module load. Any new module-level
105+ mutation of shared singletons must be reviewed against this
106+ precedent and added here.
87107
88108## Governance
89109
0 commit comments