Skip to content

Commit

Permalink
fix(observer): replace hashing with counter to imrpove performance. U…
Browse files Browse the repository at this point in the history
…I can handle rerender concerns
  • Loading branch information
samhatoum committed May 18, 2020
1 parent 129aba7 commit ef87d3b
Show file tree
Hide file tree
Showing 8 changed files with 5 additions and 119 deletions.
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ export default class Gallery {

And now any time a value inside the POJO changes, the `useObserver` hook will re-render the component. Sweet!

If the values inside the POJO do not change, the `useObserver` hook will not re-render the component. Sweet!

This is achieved internally by using `setState` with a `hash` of the POJO. You can see this in action by trying to repeatedly click the "Previous Image" button. The `previousImage` command in the `Gallery` will stop changing the `currentImage` when it gets to 0, and since the values inside the POJO are no longer changing, the `hash` method on the object ensures that the React component will not re-render.

Bonus: You can test the heck out of the interaction now without having to mess with any UI testing libraries.

### Asynchrony
Expand Down
3 changes: 0 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@
"url": "https://github.com/xolvio/pojo-observer/issues"
},
"homepage": "https://github.com/xolvio/pojo-observer#readme",
"dependencies": {
"object-hash": "^2.0.0"
},
"devDependencies": {
"@babel/cli": "^7.2.3",
"@babel/core": "^7.2.2",
Expand Down
11 changes: 0 additions & 11 deletions src/addHash.ts

This file was deleted.

66 changes: 0 additions & 66 deletions src/hash.spec.ts

This file was deleted.

11 changes: 0 additions & 11 deletions src/hash.ts

This file was deleted.

11 changes: 2 additions & 9 deletions src/useObserver.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
/* eslint-disable max-classes-per-file,react/button-has-type,no-plusplus */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import * as React from 'react'
import {FunctionComponent} from 'react'
import {render, act} from '@testing-library/react'
import useObserver from './useObserver'
import '@testing-library/jest-dom/extend-expect'
import {addHash} from './addHash'

test('add hash internally', () => {
class TestClass {
Expand Down Expand Up @@ -213,7 +211,7 @@ test('add hash explicitly', () => {
}

function ComponentUsingModel({model}: {model: TestClass}) {
const methods = useObserver(addHash(model))
const methods = useObserver(model)

return (
<div>
Expand All @@ -226,7 +224,7 @@ test('add hash explicitly', () => {
}

function OtherComponentUsingModel({model}: {model: TestClass}) {
const methods = useObserver(addHash(model))
const methods = useObserver(model)

return (
<div>
Expand Down Expand Up @@ -551,10 +549,6 @@ test('it should re-render when multi-level depth fields are set to an object who

function Component() {
useObserver(object)
console.log(
'GOZDECKI object.field.nested.deep.very',
object?.field?.nested.deep.very
)

return (
<div data-testid="foo">
Expand All @@ -581,7 +575,6 @@ test('it should re-render when multi-level depth fields are set to an object who
}
})

console.log('GOZDECKI object.field.nested.deep', object.field.nested.deep)
expect(getByTestId('foo')).toHaveTextContent('deeper')
act(() => {
object.field.nested.deep.very = 'fathoms'
Expand Down
13 changes: 3 additions & 10 deletions src/useObserver.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {useCallback, useEffect, useRef, useState} from 'react'
import hash from './hash'
import {generateId} from './helpers/generateId'

class EventEmitter {
Expand Down Expand Up @@ -260,11 +259,6 @@ function addId(model: Model): void {
})
}

function addHash(model: Model): void {
// eslint-disable-next-line no-param-reassign
if (!model.hash) model.hash = (): string => hash(model)
}

let currentId = 0

export function useUniqueId(): string {
Expand All @@ -278,11 +272,11 @@ export function useUniqueId(): string {

function useReactify(model: Model): Function {
const subscriptionId = useUniqueId()
const [, stateChange] = useState(model.hash())
const [, stateChange] = useState(0)

const stateChangeCallback = useCallback(() => {
stateChange(model.hash())
}, [model])
stateChange((prev) => prev + 1)
}, [])

useEffect(() => {
eventEmitter.on(model.__observableId, subscriptionId, stateChangeCallback)
Expand All @@ -292,7 +286,6 @@ function useReactify(model: Model): Function {
}

function decorate(model: Model): void {
addHash(model)
addId(model)
}

Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8487,11 +8487,6 @@ object-copy@^0.1.0:
define-property "^0.2.5"
kind-of "^3.0.3"

object-hash@^2.0.0:
version "2.0.3"
resolved "https://registry.yarnpkg.com/object-hash/-/object-hash-2.0.3.tgz#d12db044e03cd2ca3d77c0570d87225b02e1e6ea"
integrity sha512-JPKn0GMu+Fa3zt3Bmr66JhokJU5BaNBIh4ZeTlaCBzrBsOeXzwcKKAK1tbLiPKgvwmPXsDvvLHoWh5Bm7ofIYg==

object-inspect@^1.7.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.7.0.tgz#f4f6bd181ad77f006b5ece60bd0b6f398ff74a67"
Expand Down

0 comments on commit ef87d3b

Please sign in to comment.