Pricing single properties

[dbrudner]

No description provided.

[github-actions]

Pull Request Test Coverage Report for Build 3f7ba09ab456b51ed794d7505587844be1d5c1c7-PR-25

• 0 of 11 (0.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-2.0005%) to 48.344%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/use-checkout-pricing.js	0	11	0.0%

Files with Coverage Reduction	New Missed Lines	%
lib/use-checkout-pricing.js	1	0%

Totals
Change from base Build 24869dbc1185ce9841cdb1b6b80434e6a0dbab13:	-2.0005%
Covered Lines:	55
Relevant Lines:	114

---

💛 - Coveralls

[chrissrogers]

Would it be better to track state in a larger blob and have the handlers call into it with a spread of the current state? Unsure if that's a no-no, but this is a lot of individual state methods to keep track of.

const [billingAddress, setbillingAddress] = useState({
  country: '',
  postalCode: '',
  vatNumber: ''
});

// ...

<input
  type="text"
  value={billingAddress.country}
  onChange={e => setBillingAddress({ ...billingAddress, country: e.target.value })}
  placeholder="Country"
/>

[dbrudner]

I had considered this, but react's recommendation is to only bundle pieces of state together that update together:

we recommend to split state into multiple state variables based on which values tend to change together.

https://reactjs.org/docs/hooks-faq.html#should-i-use-one-or-many-state-variables

Having said that, my original intention was to abstract as little as possible, but the demo is definitely getting long and verbose and generally needs some love. I'm thinking a separate PR after everything else is merged in to clean up some demo-specific stuff is needed.

[chrissrogers]

👍 good notes in that doc. I think it can improve readability to merge some state together, while tending to use single state updaters for most situations. I'm mostly concerned with making the demo as readable as possible (without impacting perf unnecessarily).

Agreed that the demo could use some improvement. I'd think that there may be a UI lib we could pull in to at least do away with the CSS.

[chrissrogers]

nit: Thinking singleInputs could use a better name. rest does seem appropriate, or perhaps scalarInputs.

[dbrudner]

Totally agree, I've been thinking this the entire time as well. I picked this var name like 20 seconds into writing the feature thinking I'd think of something later and go back and update and never did. Definitely needs a better name.

[dbrudner]

d4eecff

[chrissrogers]

for flow simplicity, setLoading can move into a final then or done below these two methods.

addSubscriptions(subscriptions, checkoutPricing)
  .then(() => checkoutPricing = checkoutPricing.done(() => setPricing(checkoutPricing)))
  .catch(handleError)
  .then(() => setLoading(false));

[dbrudner]

I tried making this change, but in this case, setLoading(false) fires before setPricing is called, causing the demo to flash $0.00 before updating to its resolved value $10.00

[chrissrogers]

Will it respect the promise order if you use checkoutPricing.then instead of checkoutPricing.done ?

[dbrudner]

I just tried this, but got the same result -- sets loading to true before the promise resolves. We can pair on it tomorrow.