LG-7927 AddressSearch (draft) component relays results to USPS location search

[allthesignals]

🎫 Ticket

Link to the relevant ticket.

🛠 Summary of changes

Adds a new component, AddressSearch, which encapsulates network and UI behavior interacting with Geocoder API. Note that the ArcGIS controller sits behind a feature flag. I added arcgis_search_enabled: true to my application.yml.

Note: this is hidden behind a feature flag.

Some discussion points and TODOs:

1. Re-using request here (by exporting) introduces a circular-depenency lint error. Probably a sign that it should be extracted into a separate package, but for now was injected into the component...
2. Handling initial state: currently, on first load, it sends a blank (and likely, invalid) address object. Even optimistically this would lead to "no results." So, we should check with design/product and likely perform no querying until an address search actually happens.
   3. State management is handled here by added a foundAddress property to the usps locations dependency keys.
3. Need to hide this all behind a feature flag. @eileen-nava is working on a PR that will switch to the "real" Proofer method that performs the actual search.

Later: Hitting the enter key: I believe this is bubbling to some outer form context and triggering a submit action. I will need to investigate what that context is (if that's actually what's happening), and probably stopDefault the enter key action and/or wire it up such that it fires the "Search" button... (I'll need to look aroud to see how this is handled... I'm inclined to learn toward default form behavior (type="submit") with some configuration...)

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Added test coverage, passing
• Sandbox with feature-flagged functionality

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before:

After:

With feature flag turned on:

With feature flag turned off:

[allthesignals]

4. Need to hide this all behind a feature flag. @eileen-nava is working on a PR that will switch to the "real" Proofer method that performs the actual search.

Currently working to hide <AddressSearch/> behind a feature flag... not sure there are examples on frontend feature flags. Is this a system we support? I could use the identity-config package maybe? Unsure how that's setup per-environment...

[aduth]

To make the most of useCallback memoization, we should avoid creating a new function reference each time here, since otherwise there's no point to memoizing.

Suggested change

	<Button onClick={() => handleAddressSearch()}>Search</Button>
	<Button onClick={handleAddressSearch}>Search</Button>

[aduth]

Just want to note that this is going to be very prone to race conditions if the user submits multiple times, as the results may arrive out of order form when they were sent.

We should either cancel/nullify previous requests like we're doing in the other component, or consider a library like swc / react-query to handle this on our behalf.

[allthesignals]

To make the most of useCallback memoization, we should avoid creating a new function reference each time here, since otherwise there's no point to memoizing.

I'd like to dive deeper into understanding this because in my code I'm defining and memoizing handleAddressSearch once. The anonymous function here calls the memoized function, but React doesn't know how to track it because it's an anonymous function? Something like that?

[aduth]

Yeah, the anonymous function will create a new function each time, and because () => {} !== () => {}, React will assume that the prop value is different and that some reconciliation must happen. This runs contrary to the point of useCallback, which is to make sure the value of the callback handler remains identical (memoizedHandler === memoizedHandler) so that React could try to avoid some reconciliation effort.

IIRC the default behavior is that React will always reconcile anyways unless descendent components are marked as pure components, and our usage of React is fairly minimal (we're not a single-page app), so I typically choose not to worry too much about useCallback and friends.

Rough demo here: https://codepen.io/aduth/pen/eYKPZqL

[aduth]

The suggestion was applied, but just want to resurface the race condition concern from my other comment in this thread. I wouldn't necessarily consider it blocking, but I definitely expect it to produce some inconsistent buggy behavior.

[allthesignals]

The suggestion was applied, but just want to resurface #7385 (comment). I wouldn't necessarily consider it blocking, but I definitely expect it to produce some inconsistent buggy behavior.

@aduth 100%, yep! thank you. i pinged some folks to make sure I do that in the next PRs.

[aduth]

I'd expect these strings should be translated

[allthesignals]

@aduth yup! 100% We have a ticket for it. cc @kellular

[aduth]

For future consideration: I think it'd be valuable if addressCandidates was typed here, since currently it's any.

One way I could see this working is to accept a generic parameter to request which indicates the expected response, e.g.

Suggested change

	const addressCandidates = await request(ADDRESS_SEARCH_URL, {
	const addressCandidates = await request<AddressCandidate[]>(ADDRESS_SEARCH_URL, {

[eileen-nava]

Approved. 👍🏻

[NavaTim]

What's the effect if a network disconnect causes a Promise rejection here?

[allthesignals]

Probably something bad! I think the work we'll do in parallel on state management (Andrew's suggestion) involving SWR or something else will help standardize our network interactions...