search

rtk-incubator / rtk-query

Data fetching and caching addon for Redux Toolkit
https://redux-toolkit.js.org/rtk-query/overview
MIT License
626 stars 31 forks source link

SSR Support: delete substate.error property (Next.js getServerSideProps) #152

Closed gfortaine closed 3 years ago

gfortaine commented 3 years ago

Addresses points brought up in https://github.com/rtk-incubator/rtk-query/issues/88#issuecomment-762288424.

codesandbox-ci[bot] commented 3 years ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6e0a03836d6ae37f566b21389a7dd54a19d4b3d4:

Sandbox Source
React Configuration
React Typescript Configuration
rtk-query-demo Configuration
svelte-app-rtk-simplequery-demo Configuration
phryneas commented 3 years ago

Hmm. I'm honestly flabbergasted by the stuff next.js is pulling there, but oh well.

I'd suggest we do delete substate.error here though - not unsetting it at all would lead to pretty weird behaviour in some situations.

msutkowski commented 3 years ago

How does everyone feel about just defaulting to null instead of undefined? Is that a better long-term solution for this?

phryneas commented 3 years ago

Well, it could also just not be present yet or have never been set. And I kinda don't see the point of setting a property to null when it has never been accessed yet - especially since we would have to do that to others then as well consequently. So I'd rather just use delete X instead of X = undefined.

gfortaine commented 3 years ago

@phryneas PR updated with delete substate.error as suggested 👍

msutkowski commented 3 years ago

Thanks again for this @gfortaine! 🎉