search

Unleash / unleash-client-node

Unleash client SDK for Node.js
https://docs.getunleash.io
Apache License 2.0
210 stars 71 forks source link

fix: handle falsy context fields #629

Closed nunogois closed 2 months ago

nunogois commented 2 months ago

Follow up to https://github.com/Unleash/unleash-client-node/pull/628

I noticed we were not correctly handling falsy values in our context values due to doing a truthy check on the value when resolving it.

By replacing that check with a more explicit !== undefined check we can still properly evaluate context fields such as false or 0. I believe this check tracks with the original intent of simply checking whether the field is present.

Eventually all of this could be simplified to: return context[field]?.toString() ?? context.properties?.[field]?.toString();, but I wanted to avoid that level of refactor in this PR. I can still do it if the reviewer agrees.

Also added new tests that cover these specific cases.

coveralls commented 2 months ago

Coverage Status

coverage: 90.721% (+0.06%) from 90.658% when pulling 8b608a892fe44de59b74921d53e5bb6c5c3b5778 on fix-handle-falsy-context-fields into 0758b63bca96663a59398082c345ec828eb9ba04 on main.

chriswk commented 2 months ago

I like your suggestion of collapsing down to the optional operators. :+1:

nunogois commented 2 months ago

I like your suggestion of collapsing down to the optional operators. 👍

Addressed on a separate PR: https://github.com/Unleash/unleash-client-node/pull/630