Add changed property #14
Conversation
|
Managed to get 100% code coverage 👍 |
|
Changed |
README.md
Outdated
| - `pollInterval` - How often to re-request updated data. Pass 0 to disable polling (the default behaviour). | ||
| - `payload` - A data object to send in the request. If we are performing a GET request, it is appended into the querystring (e.g. `?keywords=hello`). If it is a POST request it is sent in the request body as JSON. | ||
| - `method` - Set the request type, either `get` or `post`. (defaults to `get`) | ||
| - `changed`: A function to call if the data actually changed during the request. |
There was a problem hiding this comment.
I think it would be good to state that no redraw will happen unless the data is changed, to make it explicit. Is there any sort of tradeoff for this? does it mean e.g. big data sets will be stored in memory twice (once by this tool and once by the calling app that uses this tool)? Does that need design consideration by whoever uses this tool?
There was a problem hiding this comment.
I've made the behaviour optional. If a change option is not specified, no comparison will be done. Also expanded in the README.
| "dependencies": { | ||
| "axios": "^0.19.0" | ||
| "axios": "^0.19.0", | ||
| "lodash.isequal": "^4.5.0" |
There was a problem hiding this comment.
seems a shame to bring any lodash in :/
Mapbox elected to use their own function: mapbox/mapbox-gl-js#5744
Worth considering? Genuine open question / mind.
There was a problem hiding this comment.
I'm bringing in just a single function though, not the whole package?
There was a problem hiding this comment.
so was mapbox. definitely prefer this to the whole lib and it's certinaly not a blocker, i just wondered if it was worth discussing
|
@marcelkornblum I added more documentation on the risks with using |
Co-Authored-By: Marcel Kornblum <marcelkornblum@users.noreply.github.com>
2 participants
None
Adding a changed property so it's easy to detect when the data changes.
Elements to look at before this is ready:
react-hooks/exhaustive-dep.