Skip to content

Add resolving state to component for multiple instances. #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/__tests__/__snapshots__/asyncComponent.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

exports[`asyncComponent in a browser environment when an error occurs resolving a component should render the ErrorComponent 1`] = `"<div>failed to resolve</div>"`;

exports[`asyncComponent in a browser environment when multiple instances of component are present should render all instances of the component 1`] = `"<div><div>Component</div><div>Component</div></div>"`;

exports[`asyncComponent in a browser environment when multiple instances of component are present should render multiple ErrorComponent 1`] = `"<div><div>failed to resolve</div><div>failed to resolve</div></div>"`;

exports[`asyncComponent in a server environment when an error occurs resolving a component should not render the ErrorComponent 1`] = `null`;
34 changes: 34 additions & 0 deletions src/__tests__/asyncComponent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import asyncComponent from '../asyncComponent'

describe('asyncComponent', () => {
const errorResolveDelay = 20
const promiseResolveDelay = 20

describe('in a browser environment', () => {
describe('when an error occurs resolving a component', () => {
Expand All @@ -21,6 +22,39 @@ describe('asyncComponent', () => {
expect(renderWrapper.html()).toMatchSnapshot()
})
})

describe('when multiple instances of component are present', () => {
it('should render all instances of the component', async () => {
const Bob = asyncComponent({
resolve: () => Promise.resolve(() => <div>Component</div>),
env: 'browser',
})
const renderWrapper = mount(
<div>
<Bob />
<Bob />
</div>,
)
await new Promise(resolve => setTimeout(resolve, promiseResolveDelay))
expect(renderWrapper.html()).toMatchSnapshot()
})

it('should render multiple ErrorComponent', async () => {
const Bob = asyncComponent({
resolve: () => Promise.reject(new Error('failed to resolve')),
ErrorComponent: ({ error }) => <div>{error.message}</div>,
env: 'browser',
})
const renderWrapper = mount(
<div>
<Bob />
<Bob />
</div>,
)
await new Promise(resolve => setTimeout(resolve, errorResolveDelay))
expect(renderWrapper.html()).toMatchSnapshot()
})
})
})

describe('in a server environment', () => {
Expand Down
16 changes: 11 additions & 5 deletions src/asyncComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ export default function asyncComponent(config) {
}

const needToResolveOnBrowser = () =>
state.module == null &&
state.error == null &&
!state.resolving &&
typeof window !== 'undefined'
state.module == null && state.error == null && typeof window !== 'undefined'

// Takes the given module and if it has a ".default" the ".default" will
// be returned. i.e. handy when you could be dealing with es6 imports.
Expand Down Expand Up @@ -89,6 +86,13 @@ export default function asyncComponent(config) {
}),
}

constructor() {
super()
this.state = {
resolving: state.resolving,
}
}

getChildContext() {
return {
asyncComponentsAncestor:
Expand Down Expand Up @@ -137,7 +141,9 @@ export default function asyncComponent(config) {
}

componentDidMount() {
if (needToResolveOnBrowser()) {
const { resolving } = this.state

if (!resolving && needToResolveOnBrowser()) {
this.resolveModule()
}
}
Expand Down