-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
State: Route query initial state is null #57610
Conversation
Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-18796 |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~15 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~1063 bytes removed 📉 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~2358 bytes removed 📉 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
fe534a4
to
a809bbd
Compare
a809bbd
to
cdbc032
Compare
Better to keep the null logic into the reducer
Closing as no longer relevant |
Changes proposed in this Pull Request
Regarding #57500 (comment), read the comments if you want to know about this approach more in detail.
This PR researches the possibility of using
null
as the initial state of route query (initial
,previous
andcurrent
fields). The use ofnull
could be a great way of describing “empty” (as knowingly empty) across all the state tree, bringing greater uniformity to the whole app.The route query state is “truly unknown” during Calypso boot, before
ROUTE_SET
action is dispatched, and so the truly semantically correct starting value should beundefined
. But there are problems betweenundefined
and Redux, and it's only for a brief moment during boot, so we could go directly withnull
.I introduced a “raw” selector (
getRouteQueryCurrent
inclient/state/route/query/selectors.js
) that simply reflects the state tree, and moved the logic for managing “null values“ up in the access layer (getCurrentQueryArguments
inclient/state/selectors/get-current-query-arguments.js
).The consumer (our components) then access data through the API, in our case the selector (
getCurrentQueryArguments
), that now acts like an _access layer over data, whose structure is obscured to the consumer.There is now a clearer distinction between the access layer, with all the business logic to guard against null values, and the data structure, that can be uniform in the use of
null
values for describingemptiness
.Read more here:
#55315 (comment)
and then #57500 (comment)
Testing instructions
Related to #55315