prescottprue / react-redux-firebase

Redux bindings for Firebase. Includes React Hooks and Higher Order Components.
https://react-redux-firebase.com
MIT License
2.55k stars 559 forks source link

Non-obvious example #926

Open gregfenton opened 4 years ago

gregfenton commented 4 years ago

On the queries page, section PARSING, the first code example:

// Works since limitToFirst and startAt are parsed into numbers
queryParams: [`limitToFirst=${limitToFirst}`, `startAt=${startAt}`, 'orderByValue'],

// COULD CAUSE UNEXPECTED BEHAVIOR!!! Values passed to limitToFirst and startAt will remain as strings (i.e not parsed)
queryParams: ['orderByValue', `limitToFirst=${limitToFirst}`, `startAt=${startAt}`],

it isn't obvious how/why these two code samples differ. Also, the fact that the prop and the variables are the same name makes it doubly less obvious what is going on here or why these two lines would behave any differently (other than the order of the array entries).

codedpills commented 4 years ago

I agree, it's hard to tell how or why those two code samples should work any differently considering they have the same params with similar values. Also at first glance, it's easy to miss the difference in positioning (assuming that matters at all).

I think in the second instance, the orderByValue param should be omitted.

// COULD CAUSE UNEXPECTED BEHAVIOR!!! Values passed to limitToFirst and startAt will remain as strings (i.e not parsed)
queryParams: [`limitToFirst=${limitToFirst}`, `startAt=${startAt}`],
prescottprue commented 4 years ago

Yeah, the example was to show that order does in fact matter, agreed that it isn't that clear

codedpills commented 4 years ago

Yeah, the example was to show that order does in fact matter, agreed that it isn't that clear

@prescottprue what happens if you don't pass the orderByValue param at all? Is limitToFirst and startAt parsed or they remain as strings?