Fixes the warning message thrown by react 0.14

[mgibeau]

This fixes the warnings in React 0.14 introduced by #572, but is not backward compatible with 0.13, which is why I've added the peerDependency declaration. My suggestion would be to create a separate release for this, eg.: 1.5.x.

[ChiefORZ]

Please read #603

[ChiefORZ]

"This error message will only be displayed in this version from react. With version 0.15.x it will no longer be displayed."

[mgibeau]

From what I understand it's actually going to break in 0.15, from the react blog

With this change, we’re deprecating .getDOMNode() and replacing it with ReactDOM.findDOMNode (see below). If your components are currently using .getDOMNode(), they will continue to work with a warning until 0.15.

[ChiefORZ]

Note that in most cases, calling findDOMNode is now unnecessary – see the example above in the “DOM node refs” section.

DOM node refs
Starting with this release, this.refs.giraffe is the actual DOM node.

That is implemented by my PR #572 as you can see:

DOMNode = this.getDOMNode() ? (this.refs[options.ref] || this).getDOMNode() : this.refs[options.ref] || this;

[mgibeau]

Yes, I understand, it's the PR I've referenced in my initial comment, but I'm not sure what the problem is, you can still keep this workaround for 0.13. I'm simply suggesting to have a separate release that is not backward compatible and avoids throwing warnings.

[RubaXa]

@mgibeau, @ChiefORZ I suggest a compromise:

DOMNode = this.refs[options.ref] || this;
DOMNode = typeof DOMNode.getDOMNode === 'function' ? DOMNode.getDOMNode() : DOMNode;

[ChiefORZ]

@RubaXa this will not get rid of the error message, because the getDOMNode method is actually availabe in this version of react (it will be removed in version 0.15 and the error message will disappear)

DOMNode = this.getDOMNode() ? (this.refs[options.ref] || this).getDOMNode() : this.refs[options.ref] || this;

is the only reliable method i could think of to

• leave Sortable.js backward compatible with react version < 0.14... but it will leave us with an error for everyone using react verion 0.14
• and without creating a seperate version of Sortable.js, which i think is unnescassary, because only us developer will see this error message!

[ChiefORZ]

i created a fiddle for it, if you would like to play around with it!

https://jsfiddle.net/q0gLex1b/

PS: Sortable React Mixin is inline in the HTML

[mgibeau]

@ChiefORZ @RubaXa I don't think it's unreasonable to just bump the (minor) version of master and make it forward compatible only. This is the advantage of using semantic versioning. Anyone on the older version can still depend on it without changing their code. This still allows you to release hotfixes in the 1.4.x branch.

The reason they changed the warnings to console.error in the latest release is mentioned in the release notes:

(Our warnings appear when you use patterns that will break in future releases and for code that is likely to behave unexpectedly, so we do consider our warnings to be “must-fix” errors.)

So, I think there is a misunderstanding with what ChiefORZ is suggesting, they will not disappear in the next version but will instead stop working.

[ChiefORZ]

@mgibeau no, it will not.
the getDOMNode method will be removed with 0.15 and the actual way to reference the DOM node this.refs[options.ref] || this will be used.
it will definitely not break and it is backward compatible!

[RubaXa]

#612 (comment)

@ChiefORZ You are wrong if they will remove 0.15 getDOMNode, the mixin will be broken, but my version will work, because I check availability before calling of this function.

[ChiefORZ]

@RubaXa yes, indeed. Thank you! getDOMNode will be undefined and the mixin will stop working...
Good sir, you will catch errors before they can appear ;)

[caub]

reading your discussion, but wondering: should we also try to make a Sortable component rather than a mixin? probably simpler and not having to put ref in options and it the jsx, I started in react so I'll give it a try

also for ReactDOM.findDOMNode(this.refs[options.ref] || this)... in react 0.14 ref returns a DOM element directly https://facebook.github.io/react/blog/2015/07/03/react-v0.14-beta-1.html#dom-node-refs, unlike before I think, so this could be simpler I think, will test

[alexander-svendsen]

Hmm does the code suggested actually work with react 0.15. As the code stands now the issue #448 will pup up again when upgrading react

[ChiefORZ]

@alexander-svendsen Unfortunateley issue #448 was never fixed...

And yes, i am positive that this code is going to work with version 0.15.

[Nic128]

Does this need more discussion or can we merge this once the merge conflicts are resolved?

[BrendanBerkley]

Uncaught TypeError: this.getDOMNode is not a function

Working with Sortable on React 15 and I get this error message. Going back to 0.14.x fixes the problem, so I'm pretty confident that this needs a new workaround! I'm still new to React so I'm not sure how to help yet.

[mgibeau]

@BrendanBerkley Unfortunately, nothing ever came out of this, so your best bet at the moment is to fork the library and apply the fix manually. I don't think this library is being maintained anymore.

[BrendanBerkley]

#754 provided the code I needed to get this working.

[RubaXa]

Dear all! Meteor is moved to the separate repository. If this issue is still actual, please create it there once again and this rojects needs a maintainer:

• ES2015+: https://github.com/SortableJS/react-sortablejs 🆘
• mixin: https://github.com/SortableJS/react-mixin-sortablejs