Skip to content

Commit

Permalink
fix: dont hot-swap registered components, #1050
Browse files Browse the repository at this point in the history
  • Loading branch information
theKashey committed Aug 21, 2018
1 parent ca3efeb commit cf165a6
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/reactHotLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getProxyById,
createProxyForType,
isTypeBlacklisted,
registerComponent,
} from './reconciler/proxies'
import configuration from './configuration'
import logger from './logger'
Expand Down Expand Up @@ -62,6 +63,7 @@ const reactHotLoader = {
}

updateProxyById(id, type)
registerComponent(type)
}
},

Expand Down
10 changes: 8 additions & 2 deletions src/reconciler/hotReplacementRender.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { PROXY_IS_MOUNTED, PROXY_KEY, UNWRAP_PROXY } from '../proxy'
import {
getIdByType,
getProxyByType,
isRegisteredComponent,
isTypeBlacklisted,
updateProxyById,
} from './proxies'
Expand Down Expand Up @@ -99,7 +100,7 @@ const equalClasses = (a, b) => {
}

export const areSwappable = (a, b) => {
// both are registered components
// both are registered components and have the same name
if (getIdByType(b) && getIdByType(a) === getIdByType(b)) {
return true
}
Expand Down Expand Up @@ -401,7 +402,12 @@ const hotReplacementRender = (instance, stack) => {
throw new Error('React-hot-loader: wrong configuration')
}

if (areSwappable(childType, stackChild.type)) {
if (
isRegisteredComponent(childType) ||
isRegisteredComponent(stackChild.type)
) {
// one of elements are registered via babel plugin, and should not be handled by hot swap
} else if (areSwappable(childType, stackChild.type)) {
// they are both registered, or have equal code/displayname/signature

// update proxy using internal PROXY_KEY
Expand Down
5 changes: 5 additions & 0 deletions src/reconciler/proxies.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { resetClassProxies } from '../proxy/createClassProxy'

let proxiesByID
let blackListedProxies
let registeredComponents
let idsByType

let elementCount = 0
Expand All @@ -16,6 +17,9 @@ export const isProxyType = type => type[PROXY_KEY]
export const getProxyById = id => proxiesByID[id]
export const getProxyByType = type => getProxyById(getIdByType(type))

export const registerComponent = type => registeredComponents.set(type, 1)
export const isRegisteredComponent = type => registeredComponents.has(type)

export const setStandInOptions = options => {
renderOptions = options
}
Expand All @@ -42,6 +46,7 @@ export const resetProxies = () => {
proxiesByID = {}
idsByType = new WeakMap()
blackListedProxies = new WeakMap()
registeredComponents = new WeakMap()
resetClassProxies()
}

Expand Down
48 changes: 48 additions & 0 deletions test/reconciler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,54 @@ describe('reconciler', () => {
expect(areComponentsEqual(second.Component, third.Component)).toBe(false)
})

it('should hot-swap only internal components', () => {
let An0
let An1
let Bn0
let Bn1
let App
{
const A = () => <div>A</div>
const B = () => <div>A</div>
App = () => (
<div>
<A />
<B />
</div>
)
An0 = A
Bn0 = B
reactHotLoader.register(App, 'App', 'test-hot-swap.js')
reactHotLoader.register(B, 'B0', 'test-hot-swap.js')
}
const wrapper = mount(
<div>
<App />
</div>,
)
{
const A = () => <div>A</div>
const B = () => <div>A</div>
App = () => (
<div>
<A />
<B />
</div>
)
An1 = A
Bn1 = B
reactHotLoader.register(App, 'App', 'test-hot-swap.js')
reactHotLoader.register(B, 'B1', 'test-hot-swap.js')
}
incrementGeneration()
wrapper.setProps({ update: 'now' })

// A-s are similar, and got merged
expect(<An0 />.type).toEqual(<An1 />.type)
// B-s are simlar, but known to be different types - not merged
expect(<Bn0 />.type).not.toEqual(<Bn1 />.type)
})

it('should regenerate internal component without AppContainer', () => {
const first = spyComponent(
({ children }) => <b>{children}</b>,
Expand Down

0 comments on commit cf165a6

Please sign in to comment.