Skip to content

Commit

Permalink
Browser Support, Memory Leak Fixes (#210)
Browse files Browse the repository at this point in the history
* added browser support to docs, cleaned up readmes, hopefully fixed `cant update state on unmounted component` memory leak

* removing images

* cleanup and standardize the caches

* cleanup
  • Loading branch information
alex-cory authored Mar 23, 2020
1 parent 6afd7e9 commit 4bdce8f
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 76 deletions.
35 changes: 20 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
<a href="http://use-http.com">
<img src="https://github.com/alex-cory/use-http/raw/master/public/dog.png" />
</a>
[![use-http logo][3]][5]

<br/>

Expand Down Expand Up @@ -875,10 +873,19 @@ Does your company use use-http? Consider sponsoring the project to fund new feat
</a>
</p>
Browser Support
---------------
If you need support for IE, you will need to add additional polyfills. The React docs suggest [these polyfills][4], but from [this issue][2] we have found it to work fine with the [`react-app-polyfill`]. If you have any updates to this browser list, please submit a PR!
| [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/edge/edge_48x48.png" alt="IE / Edge" width="24px" height="24px" />]()<br/>Edge | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/firefox/firefox_48x48.png" alt="Firefox" width="24px" height="24px" />]()<br/>Firefox | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/chrome/chrome_48x48.png" alt="Chrome" width="24px" height="24px" />]()<br/>Chrome | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/safari/safari_48x48.png" alt="Safari" width="24px" height="24px" />]()<br/>Safari | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/opera/opera_48x48.png" alt="Opera" width="24px" height="24px" />]()<br/>Opera |
| --------- | --------- | --------- | --------- | --------- | --------- | --------- |
| 12+ | last 2 versions| last 2 versions| last 2 versions| last 2 versions| last 2 versions| last 2 versions
Feature Requests/Ideas
----------------------
If you have feature requests, let's talk about them in [this issue](https://github.com/alex-cory/use-http/issues/13)!
If you have feature requests, [submit an issue][1] to let us know what you would like to see!
Todos
------
Expand All @@ -893,7 +900,6 @@ Todos
- snapshot test resources: [swr](https://github.com/zeit/swr/blob/master/test/use-swr.test.tsx#L1083), [react-apollo-hooks](https://github.com/trojanowski/react-apollo-hooks/blob/master/src/__tests__/useQuery-test.tsx#L218)
- basic test resources: [fetch-suspense](https://github.com/CharlesStover/fetch-suspense/blob/master/tests/create-use-fetch.test.ts), [@testing-library/react-hooks suspense PR](https://github.com/testing-library/react-hooks-testing-library/pull/35/files)
- [ ] maybe add translations [like this one](https://github.com/jamiebuilds/unstated-next)
- [ ] add browser support to docs [1](https://github.com/godban/browsers-support-badges) [2](https://gist.github.com/danbovey/b468c2f810ae8efe09cb5a6fac3eaee5) (currently does not support ie 11)
- [ ] maybe add contributors [all-contributors](https://github.com/all-contributors/all-contributors)
- [ ] add sponsors [similar to this](https://github.com/carbon-app/carbon)
- [ ] Error handling
Expand All @@ -908,6 +914,7 @@ Todos
- [ ] aborts fetch on unmount
- [ ] take a look at how [react-apollo-hooks](https://github.com/trojanowski/react-apollo-hooks) work. Maybe ad `useSubscription` and `const request = useFetch(); request.subscribe()` or something along those lines
- [ ] make this a github package
- [see ava packages](https://github.com/orgs/ava/packages)
- [ ] get it all working on a SSR codesandbox, this way we can have api to call locally
- [ ] make GraphQL examples in codesandbox
- [ ] Documentation:
Expand All @@ -930,16 +937,6 @@ Todos
.get()
```
- [ ] maybe add snake_case -> camelCase option to `<Provider />`. This would
convert all the keys in the response to camelCase.
Not exactly sure how this syntax should look because what
if you want to have this only go 1 layer deep into the response
object. Or if this is just out of scope for this library.
```jsx
<Provider responseKeys={{ case: 'camel' }}><App /></Provider>
```
- [ ] potential option ideas
```jsx
Expand Down Expand Up @@ -1042,3 +1039,11 @@ const App = () => {
}
```
</details>
[1]: https://github.com/alex-cory/use-http/issues/new?title=[Feature%20Request]%20YOUR_FEATURE_NAME
[2]: https://github.com/alex-cory/use-http/issues/93#issuecomment-600896722
[3]: https://github.com/alex-cory/use-http/raw/master/public/dog.png
[4]: https://reactjs.org/docs/javascript-environment-requirements.html
[5]: http://use-http.com
[`react-app-polyfill`]: https://www.npmjs.com/package/react-app-polyfill
29 changes: 25 additions & 4 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<img src="https://github.com/alex-cory/use-http/raw/master/public/dog.png" />
![use-http logo][3]

<p align="center">
<h1 align="center">useFetch</h1>
Expand Down Expand Up @@ -821,6 +821,7 @@ useFetch(options)
Who's using use-http?
=====================
<div style="display: flex; align-items: center; justify-content: center;">
<a href="https://ava.inc">
<img height="140px" src="https://github.com/alex-cory/use-http/raw/master/public/ava-logo.png" />
Expand All @@ -836,11 +837,23 @@ Who's using use-http?
</a>
</div>
Browser Support
===============
If you need support for IE, you will need to add additional polyfills. The React docs suggest [these polyfills][4], but from [this issue][2] we have found it to work fine with the [`react-app-polyfill`]. If you have any updates to this browser list, please submit a PR!
| [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/edge/edge_48x48.png" alt="IE / Edge" width="24px" height="24px" />]()<br/>Edge | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/firefox/firefox_48x48.png" alt="Firefox" width="24px" height="24px" />]()<br/>Firefox | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/chrome/chrome_48x48.png" alt="Chrome" width="24px" height="24px" />]()<br/>Chrome | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/safari/safari_48x48.png" alt="Safari" width="24px" height="24px" />]()<br/>Safari | [<img src="https://raw.githubusercontent.com/alrra/browser-logos/master/src/opera/opera_48x48.png" alt="Opera" width="24px" height="24px" />]()<br/>Opera |
| --------- | --------- | --------- | --------- | --------- | --------- | --------- |
| 12+ | last 2 versions| last 2 versions| last 2 versions| last 2 versions| last 2 versions| last 2 versions
Feature Requests/Ideas
======================
If you have feature requests, let's talk about them in [this issue](https://github.com/alex-cory/use-http/issues/13)!
<!-- Mutations with Suspense <sup>(Not Implemented Yet)</sup>
If you have feature requests, [submit an issue][1] to let us know what you would like to see!
<!--
Mutations with Suspense <sup>(Not Implemented Yet)</sup>
==================
```js
const App = () => {
Expand Down Expand Up @@ -868,4 +881,12 @@ const App = () => {
</>
)
}
``` -->
```
-->
[1]: https://github.com/alex-cory/use-http/issues/new?title=[Feature%20Request]%20YOUR_FEATURE_NAME
[2]: https://github.com/alex-cory/use-http/issues/93#issuecomment-600896722
[3]: https://github.com/alex-cory/use-http/raw/master/public/dog.png
[4]: https://reactjs.org/docs/javascript-environment-requirements.html
[`react-app-polyfill`]: https://www.npmjs.com/package/react-app-polyfill
7 changes: 4 additions & 3 deletions src/__tests__/useFetch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ describe('useFetch - BROWSER - suspense', (): void => {
afterEach((): void => {
fetch.resetMocks()
cleanup()

test.cleanup()
})

Expand Down Expand Up @@ -939,7 +940,6 @@ describe('useFetch - BROWSER - persistence', (): void => {
expect(fetch).toHaveBeenCalledTimes(1)
})


it('should have `cache` in the return of useFetch', async (): Promise<void> => {
const { result } = renderHook(
() => useFetch({ url: 'https://persist.com', persist: true }, [])
Expand All @@ -953,14 +953,15 @@ describe('useFetch - BROWSER - persistence', (): void => {
})

it('should error if passing wrong cachePolicy with persist: true', async (): Promise<void> => {

try {
const { result } = renderHook(
() => useFetch({ url: 'https://persist.com', persist: true, cachePolicy: NO_CACHE }, [])
)
expect(result.current.error).toBe(undefined)
} catch (err) {
expect(err.name).toBe('Invariant Violation')
expect(err.message).toBe(`You cannot use option 'persist' with cachePolicy: no-cache 🙅‍♂️`)
expect(err.message).toBe('You cannot use option \'persist\' with cachePolicy: no-cache 🙅‍♂️')
}

try {
Expand All @@ -970,7 +971,7 @@ describe('useFetch - BROWSER - persistence', (): void => {
expect(result.current.error).toBe(undefined)
} catch (err) {
expect(err.name).toBe('Invariant Violation')
expect(err.message).toBe(`You cannot use option 'persist' with cachePolicy: network-only 🙅‍♂️`)
expect(err.message).toBe('You cannot use option \'persist\' with cachePolicy: network-only 🙅‍♂️')
}
})
})
2 changes: 1 addition & 1 deletion src/doFetchArgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export default async function doFetchArgs<TData = any>(

// TODO: see if `Object.entries` is supported for IE
// TODO: if the body is a file, and this is a large file, it might exceed the size
// limit of the key size in the Map
// limit of the key size. Potential solution: base64 the body
// used to tell if a request has already been made
const responseID = Object.entries({ url, method, body: options.body || '' })
.map(([key, value]) => `${key}:${value}`).join('||')
Expand Down
43 changes: 21 additions & 22 deletions src/storage/localStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,32 +12,26 @@ const getCache = () => {
}
}
const getLocalStorage = ({ cacheLife }: { cacheLife: number }): Cache => {
// there isn't state here now, but will be eventually

const remove = async (name: string) => {
const isExpired = (responseID: string) => {
const cache = getCache()
delete cache[name]
localStorage.setItem(cacheName, JSON.stringify(cache))
const { expiration, response } = (cache[responseID] || {})
const expired = expiration > 0 && expiration < Date.now()
if (expired) remove(responseID)
return expired || !response
}

const has = async (responseID: string): Promise<boolean> => {
const remove = async (...responseIDs: string[]) => {
const cache = getCache()
return !!(cache[responseID] && cache[responseID].response)
responseIDs.forEach(id => delete cache[id])
localStorage.setItem(cacheName, JSON.stringify(cache))
}

const get = async (responseID: string): Promise<Response | undefined> => {
const cache = getCache()
if (!cache[responseID]) {
return
}

const { expiration, response: { body, headers, status, statusText } } = cache[responseID] as any
if (expiration < Date.now()) {
delete cache[responseID]
localStorage.setItem(cacheName, JSON.stringify(cache))
return
}
const has = async (responseID: string) => !isExpired(responseID)

const get = async (responseID: string) => {
if (isExpired(responseID)) return
const cache = getCache()
const { body, headers, status, statusText } = cache[responseID].response
return new Response(body, {
status,
statusText,
Expand All @@ -47,9 +41,8 @@ const getLocalStorage = ({ cacheLife }: { cacheLife: number }): Cache => {

const set = async (responseID: string, response: Response): Promise<void> => {
const cache = getCache()
const responseObject = await serializeResponse(response)
cache[responseID] = {
response: responseObject,
response: await serializeResponse(response),
expiration: Date.now() + cacheLife
}
localStorage.setItem(cacheName, JSON.stringify(cache))
Expand All @@ -59,7 +52,13 @@ const getLocalStorage = ({ cacheLife }: { cacheLife: number }): Cache => {
localStorage.setItem(cacheName, JSON.stringify({}))
}

return { get, set, has, delete: remove, clear }
return Object.defineProperties(getCache(), {
get: { value: get, writable: false },
set: { value: set, writable: false },
has: { value: has, writable: false },
delete: { value: remove, writable: false },
clear: { value: clear, writable: false }
})
}

export default getLocalStorage
64 changes: 37 additions & 27 deletions src/storage/memoryStorage.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,44 @@
import { Cache } from '../types'

const inMemoryStorage = new Map<string, Response | number>()
const getMemoryStorage = ({ cacheLife }: { cacheLife: number }): Cache => ({
async get(name: string) {
const item = inMemoryStorage.get(name) as Response | undefined
if (!item) return

const expiration = inMemoryStorage.get(`${name}:ts`)
if (expiration && expiration > 0 && expiration < Date.now()) {
inMemoryStorage.delete(name)
inMemoryStorage.delete(`${name}:ts`)
return
var inMemoryStorage: any = {}
var getMemoryStorage = ({ cacheLife }: { cacheLife: number }): Cache => {
const isExpired = (responseID: string) => {
const expiration = inMemoryStorage[`${responseID}:ts`]
const expired = expiration > 0 && expiration < Date.now()
if (expired) remove(responseID)
return expired
}

const get = async (responseID: string) => {
if (isExpired(responseID)) return
return inMemoryStorage[responseID] as Response
}

const set = async (responseID: string, res: Response) => {
inMemoryStorage[responseID] = res
inMemoryStorage[`${responseID}:ts`] = cacheLife > 0 ? Date.now() + cacheLife : 0
}

const has = async (responseID: string) => !isExpired(responseID)

const remove = async (...responseIDs: string[]) => {
for (const responseID of responseIDs) {
delete inMemoryStorage[responseID]
delete inMemoryStorage[`${responseID}:ts`]
}
}

return item
},
async set(name: string, data: Response) {
inMemoryStorage.set(name, data)
inMemoryStorage.set(`${name}:ts`, cacheLife > 0 ? Date.now() + cacheLife : 0)
},
async has(name: string) {
return inMemoryStorage.has(name)
},
async delete(name: string) {
inMemoryStorage.delete(name)
inMemoryStorage.delete(`${name}:ts`)
},
async clear() {
return inMemoryStorage.clear()
const clear = async () => {
inMemoryStorage = {}
}
})

return Object.defineProperties(inMemoryStorage, {
get: { value: get, writable: false, configurable: true },
set: { value: set, writable: false, configurable: true },
has: { value: has, writable: false, configurable: true },
delete: { value: remove, writable: false, configurable: true },
clear: { value: clear, writable: false, configurable: true }
})
}

export default getMemoryStorage
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export type Interceptors = {
response?: (response: Res<any>) => Res<any>
}

// this also holds the response keys. It mimics js Map
export type Cache = {
get: (name: string) => Promise<Response | undefined>
set: (name: string, data: Response) => Promise<void>
Expand Down
11 changes: 7 additions & 4 deletions src/useFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ function useFetch<TData = any>(...args: UseFetchArgs): UseFetch<TData> {
const hasMore = useRef(true)
const suspenseStatus = useRef('pending')
const suspender = useRef<Promise<any>>()
const mounted = useRef(false)

const [loading, setLoading] = useState<boolean>(defaults.loading)
const forceUpdate = useReducer(() => ({}), [])[1]
Expand Down Expand Up @@ -78,19 +79,19 @@ function useFetch<TData = any>(...args: UseFetchArgs): UseFetch<TData> {
interceptors.request
)

if (!suspense) setLoading(true)
if (!suspense && mounted.current) setLoading(true)
error.current = undefined

if (response.isCached && cachePolicy === CACHE_FIRST) {
try {
res.current = response.cached as Res<TData>
res.current.data = await tryGetData(response.cached, defaults.data)
data.current = res.current.data as TData
if (!suspense) setLoading(false)
if (!suspense && mounted.current) setLoading(false)
return data.current
} catch (err) {
error.current = err
setLoading(false)
if (mounted.current) setLoading(false)
}
}

Expand Down Expand Up @@ -134,7 +135,7 @@ function useFetch<TData = any>(...args: UseFetchArgs): UseFetch<TData> {
controller.current = undefined
}

if (!suspense) setLoading(false)
if (!suspense && mounted.current) setLoading(false)

return data.current
} // end of doFetch()
Expand Down Expand Up @@ -182,12 +183,14 @@ function useFetch<TData = any>(...args: UseFetchArgs): UseFetch<TData> {

// onMount/onUpdate
useEffect((): any => {
mounted.current = true
if (Array.isArray(dependencies)) {
const methodName = requestInit.method || HTTPMethod.GET
const methodLower = methodName.toLowerCase() as keyof ReqMethods
const req = request[methodLower] as NoArgs
req()
}
return () => mounted.current = false
// TODO: need [request] in dependency array. Causing infinite loop though.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, dependencies)
Expand Down

0 comments on commit 4bdce8f

Please sign in to comment.