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

Refactor render arrow functions #969

Merged
merged 1 commit into from
Aug 2, 2017

Conversation

warlo
Copy link
Contributor

@warlo warlo commented Aug 2, 2017

Having debugged a problem while using server rendering for two days I figured that the render functions in slate does not work nicely with react-prepare's component check. The reason is that when using the render() {} method it transpiles to

  _createClass(Test, [{
    key: "render",
    value: function render() {}
  }]);

and is therefore possible to determine the render property on the prototype. While render = () => transpiles to this.render = function () {}; and results in undefined render on the prototype. This makes _callClassCheck raise TypeError("Cannot call a class as a function");, since this is undefined due to no preparation of the component in react-prepare.

Babel transpile examples are provided here

I've done some research and haven't really found much usage of the render as arrow functions, is this done by intention? My understanding was that arrow functions usually are used for helper methods and event handlers in components for implicit .bind(), but not for the render method itself.

One could argue that this is something that should be accepted and improved in react-prepare, but I think that slate should use the most common practice.

@ianstormtaylor
Copy link
Owner

Fair enough @hanswilw, thanks! I only did the arrow function version because it was easier to not have to distinguish between when to use arrows and when not. But this does seem more "correct", so I'm happy to change.

@ianstormtaylor ianstormtaylor merged commit 469d8b3 into ianstormtaylor:master Aug 2, 2017
chemzqm pushed a commit to chemzqm/slate that referenced this pull request Aug 3, 2017
oyeanuj added a commit to oyeanuj/slate that referenced this pull request Aug 11, 2017
* ianstormtaylor/master: (61 commits)
  0.21.2
  alphabetize package.json scripts
  Speed up getting blocks at a range
  Remove instanceOf checks to allow Slate objects to be identifiable across module instances (ianstormtaylor#930)
  Refactor render arrow functions (ianstormtaylor#969)
  update prosemirror description in readme
  update prosemirror description in docs
  update prosemirror comparison in readme
  0.21.1
  Fix for HTML pasting not working in IE (ianstormtaylor#882)
  Remove unneeded check. (ianstormtaylor#961)
  Upgrade react-frame-component. (ianstormtaylor#962)
  adding forced-layout-example (ianstormtaylor#954)
  Reckon with inconsistencies between parse5 and native DOMParser (ianstormtaylor#952)
  Omit version on gzip size badge (ianstormtaylor#947)
  update issue template and contributing docs
  add reboo editor to resources
  add showcase to resources
  0.21.0
  update changelog
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants