Skip to content
This repository has been archived by the owner on Mar 6, 2023. It is now read-only.

Add request promise resolver #4

Merged
merged 4 commits into from
Mar 9, 2017
Merged

Conversation

simenbrekken
Copy link
Contributor

@simenbrekken simenbrekken commented Mar 3, 2017

This adds a new module resolver which is capable of resolving promises recursively without an external context.

Usage example:

const renderApplication = () => renderToString(<Application />)

try {
  const markup = await resolveRequestPromises(renderApplication, { maxIterations: 2 })

  res.send(markup)
} catch (error) {
  next(error)
}

Closes #2 and #3

This still needs to be documented properly along with examples for which I've created a separate issue #5

src/index.js Outdated
// for symbols to show up in actions as JSON.stringify strips symbols
export const FETCH = 'fetch' // Symbol('fetch')
export { getActionRequestOptions, getRequestState } from './utils'
export { registerRequestPromise, resolveRequestPromises } from './resolver'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why export registerRequestPromise? What is the use-case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, removed in f1b52fd

src/resolver.js Outdated
return Promise.resolve(result)
}

export const registerRequestPromise = createRequestPromise => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

registerRequestPromise = promise => {
  if (resolving && promise) {
    promises.push(promise)
  }
}
// fetchData.js
componentWillMount() {
  registerRequestPromise(this.dispatchIfNeeded(this.props))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the promise shouldn't be created unless we are actually resolving.still create a promise even if it shouldn't

@simenbrekken simenbrekken merged commit 4d1d9c5 into master Mar 9, 2017
@einarlove einarlove deleted the resolve-dependencies branch March 9, 2017 09:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React Router v4
2 participants