Use data attribute names that are less collision-prone

[colllin]

What are the chances we could change the names of the data attributes to something that has a better chance of not causing headaches for other developers.

It doesn't seem right to both attempt to make the javascript handling of "remote links" totally unobtrusive and magical, but also place the hidden logic right in the path where you are likely to collide with it.

How about data-rails-ujs-remote, data-rails-ujs-confirm, data-rails-ujs-method as defaults from now on? That way you would have to go out of your way to collide with it. Especially for 'data-method', that's pretty likely to be the first data attribute name you come up with if you're writing your own custom handling for a remote link. It's surprising and not easy to debug what's going on when your custom handling doesn't work the way you expect it to, because of a name collision that you didn't write and wouldn't even find if you searched your application for it.

[PikachuEXE]

I agree on the idea of namespacing the data attribute but the name could be shorter like data-rails-method or data-ujs-method No need to use the full name if we don't have to

But I guess this won't be implemented since it affects the core rails too

[JangoSteve]

I'm not completely against namespacing the data attributes, to be honest. But it would be something that'd have to be coordinated with the back-end team, since we'd need to change where the view helpers point. But the view helper names themselves could remain the same, so most of the rails remote documentation wouldn't become stale.

[PikachuEXE]

Maybe that should be changed in rails 4.1 :)

[rafaelfranca]

This will be trick. We changed all the Rails methods to avoid things like link_to "foo", "/foo", remote: true to link_to "foo", "/foo", data: { remote: true }. We choose that path because we are making explicit the remote part is being treated using JavaScript through this data attribute.

If we rename the data attributes of this plugin we will break a lot of people application.

[rafaelfranca]

Opss. remote was not changed. The only thing we changed as confirm and disabled_with. So these two will not easy to add namespace.

I think we can change here and I'll gladly change in the Rails itself but we can't permit Rails 4.0 applications to install the newest version of jquery_ujs or the applications will not work.

[colllin]

@rafaelfranca Thanks for mentioning the data-confirm changes. I didn't know about those. That would be the other solution to the problem - have people explicitly specify link_to "Foo", '/foo', data: {method: :delete}. Then it's impossible to write a collision from the ruby side. Compared to the namespace suggestion which only makes collisions less likely.

[rafaelfranca]

collisions are already made in the Ruby side. If you use

link_to "foo", '/foo, method: :delete, data: { method: 'foo' }

Only one of the values will be used.

[colllin]

Exactly, that's what I'm trying to say. If the only way to activate the ujs library is by specifying the data attribute yourself (using data: {method: :delete} or similar) then it's impossible to collide in the way you just showed, unless I guess if you do something silly like data: {method: :delete}, 'data-method' => 'foo'. But that would be quickly sorted out on stackoverflow without having to post mountains of code. And would promote the understanding of data attributes and ujs in general.

On Friday, November 1, 2013, Rafael Mendonça França wrote:

collisions are already made in the Ruby side. If you use

link_to "foo", '/foo, method: :delete, data: { method: 'foo' }

Only one of the values will be used.

— Reply to this email directly or view it on GitHubhttps://github.com/rails/jquery-ujs/issues/341#issuecomment-27561319 .

[JangoSteve]

If I understand correctly, the collisions you were referring to weren't with if you specify the same data attribute multiple times in your ruby helper, but rather potential collisions with other libraries that could use data-method, data-confirm, etc. for other purposes, no?

[PikachuEXE]

@JangoSteve From what I understand, Yup

[colllin]

@JangoSteve Yes, totally.

So there are really 2 problems: 1 - Possible name collisions 2 - The name is connected to the view via MAGIC

Sounds like they're already fixing (already fixed?) #2 for data-confirm. But now that I think about it, making you manually specify data: {method: :delete} in your view (instead of method: :delete) would help awareness but wouldn't have saved me from the name collision.

Something that might have helped me, besides having a less collision-prone data attribute name, would be maybe adding a config setting to specify the data attribute name? then i would have found the default value when i searched for 'method'. Something like that might be necessary anyway to help with migration ?

Maybe the best idea is to chalk this up to "Best practice is to namespace your data attributes." And then expect people to go to extra lengths to namespace all of their own ujs data attributes. (not a bad idea). and then I will follow suit.