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

ADD async render support #1253

Closed
wants to merge 3 commits into from
Closed

ADD async render support #1253

wants to merge 3 commits into from

Conversation

ndelangen
Copy link
Member

Issue: -add is sync but sometimes we want to do some async setup on first render-

What I did

Add support for a function in .add() that returns a Promise

Todo

  • Add this to docs

How to test

Should add something to the kitchen sink probably.

@codecov
Copy link

codecov bot commented Jun 11, 2017

Codecov Report

Merging #1253 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1253    +/-   ##
========================================
  Coverage   14.35%   14.35%            
========================================
  Files         201      201            
  Lines        4612     4612            
  Branches      499      614   +115     
========================================
  Hits          662      662            
+ Misses       3522     3426    -96     
- Partials      428      524    +96
Impacted Files Coverage Δ
app/react/src/client/preview/render.js 0% <0%> (ø) ⬆️
app/react/src/client/preview/client_api.js 39.28% <0%> (ø) ⬆️
app/react/src/client/preview/init.js 0% <0%> (ø) ⬆️
addons/info/src/components/PropVal.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/types/Object.js 5.81% <0%> (ø) ⬆️
app/react/src/client/preview/config_api.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/api/actions/api.js 49.42% <0%> (ø) ⬆️
addons/info/src/components/Node.js 0% <0%> (ø) ⬆️
app/react/src/server/babel_config.js 44.82% <0%> (ø) ⬆️
app/react/src/client/preview/reducer.js 0% <0%> (ø) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33096d4...9622a8e. Read the comment docs.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward. But needs:

  • docs including a motivating example
  • test case

const error = {
title: `Expecting a React element from the story: "${selectedStory}" of "${selectedKind}".`,
description: stripIndents`
return Promise.resolve(story(context)).then(element => {
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be nice to have an example usage of this somewhere.

Copy link
Member Author

@ndelangen ndelangen Jul 5, 2017

Choose a reason for hiding this comment

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

Good point!, thanks for the review 👍

@ndelangen
Copy link
Member Author

I'm dropping this for now, and will revisit this at a later stage.
#1209

@ndelangen ndelangen closed this Jul 28, 2017
@ndelangen
Copy link
Member Author

Please do NOT remove remote branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants