Revamp local state tutorial chapter for AC3

[StephenBarlow]

No description provided.

[benjamn]

@StephenBarlow Really glad to see this documentation getting modernized. Feel free to take or leave any of these suggestions!

[benjamn]

Suggested change

	gql // highlight-line
	gql, // highlight-line

Do we have a style policy about trailing commas? I don't have any strong feeling; just wanted to check.

[StephenBarlow]

We don't, largely because I'm not 100% clear yet on where they're desirable and where they break everything 😄

But I'll add this one!

[benjamn]

Suggested change

	const client: ApolloClient<NormalizedCacheObject> = new ApolloClient({
	const client = new ApolloClient({

I don't think the type parameter is necessary here? If that makes sense, you can also remove NormalizedCacheObject from the import above.

[StephenBarlow]

I'm a bit clueless on that point; this is a TypeScript sample that also auto-generates the vanilla JS sample, and my TS chops aren't sufficient to say when this typing is a good and necessary practice versus when it's superfluous 😬

[benjamn]

I like this syntax, so I think we should keep it the way it is, but the following also works:

read: () => isLoggedInVar(),

However, this won't work, because the read function is called with multiple arguments:

read: isLoggedInVar,

[benjamn]

Suggested change

	// Clear localStorage
	localStorage.clear();
	// Remove token from localStorage
	localStorage.removeItem('token');

I know you didn't add this code, but I think clearing localStorage is probably a little too aggressive?

[StephenBarlow]

Agreed, will make corresponding change in "final" version of repo

[benjamn]

Suggested change

	id: `User:${localStorage.getItem('userId')}`,
	id: cache.identify({
	__typename: 'User',
	id: localStorage.getItem('userId'),
	}),

This avoids manual string interpolation, and reuses the same keyFields or dataIdFromObject logic that the user may have configured for the User type.

[StephenBarlow]

Ahh I'm realizing that I need to make some changes to previous chapters to get userId stored locally again. That'll take some time tomorrow.

[benjamn]

Suggested change

	> The `cache.modify` method is a powerful and flexible tool for interacting with cached data. To learn more about it, see [`cache.modify`](https://www.apollographql.com/docs/react/caching/cache-interaction/#cachemodify)
	> The `cache.modify` method is a powerful and flexible tool for interacting with cached data. To learn more about it, see [`cache.modify`](https://www.apollographql.com/docs/react/caching/cache-interaction/#cachemodify).

Just a trailing period.

[benjamn]

Suggested change

	const ToggleTripButton: React.FC<ActionButtonProps> = ({ id }) => {
	const cartItems = cartItemsVar();
	const isInCart = id ? cartItems.includes(id) : false;
	const [, toggleTrip] = useState(isInCart);
	return (
	<div>
	<Button
	onClick={() => {
	if (id) {
	cartItemsVar(
	isInCart
	? cartItems.filter((i) => i !== id)
	: [...cartItems, id]
	);
	// Trigger a re-render to pick up the `cartItemsVar` changes.
	toggleTrip(!isInCart);
	}
	}}
	data-testid={'action-button'}
	>
	{isInCart ? 'Remove from Cart' : 'Add to Cart'}
	const ToggleTripButton: React.FC<ActionButtonProps> = ({ id }) => {
	const cartItems = useReactiveVar(cartItemsVar);
	const isInCart = id ? cartItems.includes(id) : false;
	return (
	<div>
	<Button
	onClick={() => {
	if (id) {
	cartItemsVar(
	isInCart
	? cartItems.filter(itemId => itemId !== id)
	: [...cartItems, id]
	);
	}
	}}
	data-testid={'action-button'}
	>
	{isInCart ? 'Remove from Cart' : 'Add to Cart'}

This can be a bit simpler (no need for useState) if you use useReactiveVar.

As a side note, it might be a good idea to reevaluate cartItemsVar() inside the onClick handler, so you aren't modifying a stale value. However, in this example, I don't think that causes any problems in practice.