fix(buildSrcSet): ensure operation is idempotent #167
Conversation
There was a problem hiding this comment.
Looks good @ericdeansanchez! Only a few small things, overall looks great
|
No change needed and it's not a big thing, but we don't usually include the changes to |
|
Also it looks like you're going to need to rebase onto main before merging to keep the history clean 😄 |
|
Thanks for the great feedback! I think what happened with |
The purpose of this PR is to ensure the buildSrcSet operation is idempotent. Prior to this PR, code modified the input-parameters, params, causing identical calls to buildSrcSet to produce different results. Now, the input params are copied into a new queryParams object and this object is passed to callers requiring params. A test has been written to show that the behavior has changed from the behavior detailed in issue #158––the input params object remains unchanged after calling buildSrcSet and that calling buildSrcSet multiple times produces the same result (given the same inputs). I also ripgrep'd through the repo with `rg 'params.[:alpha:]'` and with `rg 'params\['` to ensure params is never mutated (i.e. it never appears on the left hand side of any expression). Closes #158
ericdeansanchez
force-pushed
the
mitigate-mutation
branch
from
1ae3a79 to
a89e6ed
Compare
|
I'm closing this PR in favor of In the above PR, I tried to use git to clean up some commits, but there were a couple casualty-commits (that were undone unfortunately 🙃. |
The purpose of this PR is to ensure the
buildSrcSetoperation isidempotent. Prior to this PR, code modified the input-parameters,
params, causing identical calls tobuildSrcSetto produce differentresults.
Now, the input
paramsare copied into a newqueryParamsobject andthis object is passed to callers requiring
params. A test has beenwritten to show that the behavior has changed from the behavior detailed
in issue #158––the input
paramsobject remains unchanged after callingbuildSrcSetand that callingbuildSrcSetmultiple times producesthe same result (given the same inputs).
I also ripgrep'd through the repo with
rg 'params.[:alpha:]'and withrg 'params\['to ensureparamsis never mutated (i.e. it neverappears on the left hand side of any expression).
Closes #158