Split new CodeEvaluator component out from Preview #976
Conversation
Codecov Report
|
|
Hmm... I increased coverage over original code. I'm guessing your coverage standards have increased. I'll have a go of adding more tests. |
|
|
||
| state = this.props.initialState; | ||
| setStateBinded = this.setState.bind(this); | ||
| /* eslint-disable no-invalid-this */ |
There was a problem hiding this comment.
Yes, in the handleError function.
| code={code} | ||
| evalInContext={this.props.evalInContext} | ||
| onError={this.handleError} | ||
| compilerConfig={this.context.config.compilerConfig} |
There was a problem hiding this comment.
Shouldn't CodeEvaluator read config from context itself, similar to what you did for the Editor?
There was a problem hiding this comment.
I don't think it inherits context when it is rendered via ReactDOM.render(wrappedComponent, this.mountNode);
| @@ -0,0 +1,94 @@ | |||
| import React, { Component } from 'react'; | |||
There was a problem hiding this comment.
Do you have any new code here? If not I won't read the whole file ;-)
This is React specific, so we should reflect it in the name: something like ReactExample (not the best too but I have no better ideas ;-)
There was a problem hiding this comment.
Oh, right... yeah, git isn't being your friend here, this is all moved from Preview.
The only things I changed were:
- I renamed
PreviewComponenttoInitialStateComponent - I passed in
compilerConfigas a prop instead of reading it offcontextas discussed above.
I'm happy to rename it... I struggled to come up with a decent name to.
|
|
||
| // Wrap everything in a React component to leverage the state management | ||
| // of this component | ||
| class InitialStateComponent extends Component { |
There was a problem hiding this comment.
Something like StateHolder would be more correct: it's not just initial state here.
There was a problem hiding this comment.
Gotcha, I'll change it.
| code={code} | ||
| evalInContext={this.props.evalInContext} | ||
| onError={this.handleError} | ||
| compilerConfig={this.context.config.compilerConfig} |
|
Oops! Sorry, used to a precommit hook that runs tests. Will fix this now. |
|
Thanks, merged! |
Fixes #975
Created new
CodeEvaluatorcomponent, by extracting it fromPreview. This separates the concerns/responsibilities and also exposes theCodeEvaluatorfor use in plugins.