Hylozoic / hylo-evo

Hylo UI
Apache License 2.0
36 stars 10 forks source link

Remove get() and curry() calls from getRouteParam #1609

Closed tibetsprague closed 3 weeks ago

tibetsprague commented 3 months ago

get can be replaced by ?. and curry seems like its not being used? will have to test everywhere where getRouteParam is called

KevinTriplett commented 3 months ago

May I also remove the unused state argument? Which means removing the second argument from all the calls.

tibetsprague commented 3 months ago

Sure On Apr 9, 2024, at 8:09 PM, Kevin Triplett @.***> wrote: May I also remove the state argument, since it's not used -- which means removing all the nulls from the calls.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

KevinTriplett commented 3 months ago

Thanks -- I'm a bit lazy, and I may not be able to test for every case, so I did a regex search for any call to getRouteParam that used less than 3 arguments, which would indicate use of the curried function. Results:

getRouteParam\([^,]+,[^,]+,[^,]+,[^,]+\) matches 3 (this is where we pass in false for warn to suppress console.log warning message getRouteParam\([^,]+,[^,]+,[^,]+\) matches all except the above 3 getRouteParam\([^,]+,[^,]+\) no matches getRouteParam\([^,]+\) matches only the console.log warning (see below line) (where [^,] matches any character except comma)

const getRouteParam = curry((key, state, props, warn = true) => {
  if (warn && !props.match) console.warn(`getRouteParam('${key}') missing props.match`). <=== matches this line
  return get(['match', 'params', key], props)
})

I share your concern, and it's not like we're fixing a bug. I prefer to remove these for cleaner code and less moving parts. Your call, I'll trust you and I'm fine with walking away from this issue. :)