You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A developer on my current project had the task of building out a core table for us to reuse. The code was clean; everything worked great; overall we are very happy with this table (compared to some tables I've used during my 7-year love affair with React).
A bug came up and the cause wasn't quite evident.
His nextPage button was sometimes broken -- previousPage worked fine; nextPage would fail, but only if not preceded by a previousPage). The cause was 2-fold (well, 3 fold if you count the fact that TS probably would have identified the issue immediately -- this particular project isn't one of our TS projects).
His goToPage Select component was using event.target.value (a string), which works fine (but, sets paginationTableState's pageIndex to a string value, like "5").
previousPage and nextPage setPageIndex internal functional update doesn't coerce that current page state value to a base-10 number.
I just looked to see the type on pageIndex in PaginationState, and sure enough, pageIndex: number (so we would have gotten a warning about this on our Typescript projects), but would there be harm in coercing old to a number in gotoPage, or even to take all functionalUpdaters, when executed, and coerce the returned value into a base 10 number? (nobody is using HEX values for page indexes are they?)
The overloading of the + in JS was the reason previousPage worked and nextPage didn't:
Again... all of this isn't an issue for typescript projects, and it did cause some good discussions with our dev team, but I thought I'd point it out, and wouldn't mind hearing some thoughts on the functionalUpdate pattern on this project.
Love the code when glancing through it -- so easy to read and understand quickly. Great job!!
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
A developer on my current project had the task of building out a core table for us to reuse. The code was clean; everything worked great; overall we are very happy with this table (compared to some tables I've used during my 7-year love affair with React).
A bug came up and the cause wasn't quite evident.
nextPage
button was sometimes broken -- previousPage worked fine; nextPage would fail, but only if not preceded by a previousPage). The cause was 2-fold (well, 3 fold if you count the fact that TS probably would have identified the issue immediately -- this particular project isn't one of our TS projects).His
goToPage
Select component was using event.target.value (a string), which works fine (but, sets paginationTableState's pageIndex to a string value, like "5").previousPage
andnextPage
setPageIndex internal functional update doesn't coerce that current page state value to a base-10 number.I just looked to see the type on pageIndex in PaginationState, and sure enough,
pageIndex: number
(so we would have gotten a warning about this on our Typescript projects), but would there be harm in coercingold
to a number in gotoPage, or even to take all functionalUpdaters, when executed, and coerce the returned value into a base 10 number? (nobody is using HEX values for page indexes are they?)The overloading of the
+
in JS was the reason previousPage worked and nextPage didn't:console.log('2' + 1); // output = 21
console.log('2' - 1); // output = 1
Again... all of this isn't an issue for typescript projects, and it did cause some good discussions with our dev team, but I thought I'd point it out, and wouldn't mind hearing some thoughts on the functionalUpdate pattern on this project.
Love the code when glancing through it -- so easy to read and understand quickly. Great job!!
Beta Was this translation helpful? Give feedback.
All reactions