Skip to content
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

[WIP] Make createNode and createPage actions async, replace pluginRunner #12202

Closed

Conversation

stefanprobst
Copy link
Contributor

@stefanprobst stefanprobst commented Mar 1, 2019

This is to get feedback, not for merging.

The proposal is to make the createNode and createPage actions async (with redux-thunk), which would allow

  • awaiting the action, so the caller can know when all cascading actions have finished. This is important to be able to use createNode in field resolvers added with the new createResolvers API. One usecase would be to extend fields on types from third-party schemas, and use createRemoteFileNode there. This does not currently work, because the field resolver has no way of knowing when all cascading actions triggered by createNode are done, and it can return. See e.g. here for an example what I'm thinking of.
  • and to remove pluginRunner and the running-APIs-tracking logic in apiRunner

NOTE: Most of the stuff in this PR is noise from changing action calls in examples and plugins, the imporant parts are just in actions and api-runner.

@stefanprobst stefanprobst requested review from a team as code owners March 1, 2019 07:41
@wardpeet
Copy link
Contributor

wardpeet commented Mar 1, 2019

@pieh or @KyleAMathews are probably best to answer but making things async isn't always as easy as it sounds.

@stefanprobst
Copy link
Contributor Author

Very true! I can build www with it though.

resolve(apiRunInstance.results)
}
const { emitter } = require(`../redux`)
emitter.emit(`API_RUNNING_QUEUE_EMPTY`)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about scenario like this:

exports.sourceNodes = async ({ actions }) => {

  await createNode(someNode)

  await new Promise(resolve {
    // simulate long running async operation
    setTimeout(() => {
      createNode(someOtherNode)
    }, 5000)
  })
}

The API_RUNNING_QUEUE_EMPTY would be emitted after creating someNode and running transformers for it - but sourceNodes is not completed yet and we shouldn't emit yet. I think we still need apiRunningById (or similar) tracker to make sure we emit only if there are no APIs running

@stefanprobst
Copy link
Contributor Author

@pieh Yes, API_RUNNING_QUEUE_EMPTY being emitted too early is an issue. But this could be easily solved by a simple counter like here (I think).

The bigger plan with this though would be to step-by-step move all our event-emitter-based code into redux action creators (this does this for plugin-runner only). I find it's a bit confusing that we are currently using both redux and events (and re-emit every action as an event). One advantage with having a Promise chain of cascading API calls that can be awaited would also be that we could try a different approach to hot reloading that is more explicit. Maybe I should expand all of this in a new PR. As for something actionable in Gatsby v2 world, how do you feel about something along the lines of the first commit? It's not pretty, but it should allow createRemoteFileNode in resolvers.

@stefanprobst stefanprobst deleted the create-node-awaits-transforms branch July 8, 2019 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants