facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.89k stars 46.51k forks source link

Stop doing data-*, aria-*, start using dataSet #1259

Open zpao opened 10 years ago

zpao commented 10 years ago

The DOM already exposes data-* as dataset but it's doing transformation from hyphenated to camelCase. From MDN:

<div id="user" data-id="1234567890" data-user="johndoe" data-date-of-birth>John Doe
</div>

var el = document.querySelector('#user');

// el.id == 'user'
// el.dataset.id === '1234567890'
// el.dataset.user === 'johndoe'
// el.dataset.dateOfBirth === ''

el.dataset.dateOfBirth = '1960-10-03'; // set the DOB.

// 'someDataAttr' in el.dataset === false

el.dataset.someDataAttr = 'mydata';
// 'someDataAttr' in el.dataset === true

We should just start supporting dataSet (because camelCase). This will allow a couple things:

We'll want to do the reverse of what the DOM is doing. eg <div dataSet={{dateOfBirth: 'val', foo: 'bar'}} /> becomes <div data-date-of-birth="val" data-foo="bar"></div>.

To the best of my knowledge, aria-* doesn't have a corresponding API, but we should make it work the same way. I think ariaSet makes sense.

kmeht commented 10 years ago

Cool, I'll take a look at this this week.

chenglou commented 10 years ago

Last point, assuming you mean dataset = {...}, wouldn't work. dataset is a DOMStringMap and setting it an obj doesn't do anything. So if we decide to do this it'll likely be sharing the logic with style. #1543.

jackdclark commented 9 years ago

Hi @zpao, is this still on the radar?

zpao commented 9 years ago

Still on the radar but pretty low priority. I'd be interested to see if it could be revived - @chenglou has the most context on why we didn't quite follow through (I think there were memory tradeoffs)

chenglou commented 9 years ago

Yeah, pretty low-priority I think:

gaearon commented 9 years ago

People rarely use data-* in react now, maybe?

I've written a ton of React code this year and I don't remember ever using it. Maybe no point making it easier.

syranide commented 9 years ago

Regardless of whether or not it is rarely used I think data taking an object like style is the correct approach (that's the representation in JS too). But aria are really just prefixed attributes and it seems like it would be detrimental to have them be part of an object.

jackdclark commented 9 years ago

Our use case is using the data attributes for various tracking libraries that we use in our applications. These libraries look for these attributes & set up their own listeners. I wanted to be able to pass an object in e.g.

// this example
this.props.data = {
  track-action: 'click',
  track-location: 'basket'
};
...
<button data={this.props.data} />

// would render
<button data-track-action="click" data-track-location="basket"></button>

Our current workaround is react-data-attributes-mixin. We might be missing something obvious that we could be using instead?

syranide commented 9 years ago

@jackdcrawford <button {...{'data-track-action': ...}} /> should work just fine.

jimfb commented 9 years ago

@syranide :+1:

radubrehar commented 7 years ago

@syranide @gaearon I think React needs to make dataSet work as expected. There are use-cases for this, even if the vast majority of people is not using this a lot.

For example, we're implementing a very powerful tooltip for React. When the user hovers over a tooltip-able element, we display a tooltip with some content - it would be useful for the tooltip to know the React element/component that it targets, or at least the props accessible at that point. Yet React does not have any way the user can get a React element from an HTMLElement reference (without ref) - and this is okay - but at least we could access some props the user can put in the dataSet or even put there a React element

<a data-tooltip={<div> my custom jsx tooltip</div>>tooltip target</a>

So now when the user hovers over a [data-tooltip] html element (the tooltip target), we could take target.dataSet.tooltip and just make that the contents of the tooltip - now we currently only support data-tooltip="...string..." and inject that as the tooltip html. Or we could do <a data-tooltip-props={...}>tooltip target</a> and make the tooltip read dataSet.tooltipProps as an object.

I am aware this could break context, but it could be a starting point. What do you think?

syranide commented 7 years ago

@radubrehar Why not implement the tooltip in React instead? Also, you cannot store objects in dataset. Using ref you can do what you're trying to do here, attaching data to the DOM nodes (even a reference to the React component) that you can use however you want.., I wouldn't recommend it but that's certainly possible.

radubrehar commented 7 years ago

@syranide thanks for the quick update. I'll try to briefly explain our scenario:

We want to implement a tooltip that users can just render somewhere in their react component tree and forget about it.


<App>
   <Tooltip target="[data-tooltip]">
  {(targetNode) => {
          return ... // render tooltip content here, based on the current targetNode
  }}
   </Tooltip>
   <Header />
   <AppBody> ... <AppBody />
  <Footer />
</App>

So we want the tooltip to be displayed only for HTMLElements that match the "[data-tooltip]" selector - so all those elements share the same tooltip. There are two rendering strategies:

We don't want to force users of the tooltip add any other refs to their code - and if they use the first rendering strategy, not even a data-tooltip.

Using function as a child - we want to call the function with the tooltip target node (the HTMLElement reference) AND, if it were possible, with the React element/component behind it. But there is no public API/reliable way to get that from a HTMLElement reference.

At this point, our thought was to use dataSet - if React were to implement this properly. In this way, the users could do something like:

<a data-tooltip={<ul> ... <li /> ... <li /> ... <ul />} >tooltip target 1</a>
<a data-tooltip={<ul> ... <li /> ... <li /> ... <ul />} >tooltip target 2</a> 

<Tooltip target="[data-tooltip]">
{(targetNode) => {
  return targetNode.dataSet.tooltip
}}
</Tooltip>

And this would do the trick.

Would be nice to also have the following

<Tooltip target="[data-tooltip]">
{(targetNode, reactElement) => {
  const props = reactElement.props
  // ... get some important data from the underlying react element props
  // and do some rendering based on it
}}
</Tooltip>

But this not an issue that we cant get the underlying React element behind an HTMLElement without ref, if the dataSet were to work as expected.

Hope this is not too long and too specific - and that React gets some real benefits from implementing dataSet :)

Update

@syranide my code assumptions from above were based on the ability of storing any js value inside dataset - thanks for eye opening on this - digging into this right now.

So doing <a data-tooltip={<ul /> }>tooltip target</a> is definitely not an option then.

radubrehar commented 7 years ago

I think this will be possible when/if custom attributes land in React

quantizor commented 7 years ago

@radubrehar Not without special handling. There's a comment above stating that you can't directly provide an object to element.dataset.

aditya2272sharma commented 7 years ago

bump

May I ask, since when eliminating native HTML properties became acceptable for frameworks?

People rarely use data-* in react now

I'd blame the inexperience of the React community/programmers for the myopia. React doesn't constitute whole of web. And veterans (yeah, jQuery programmers) would know data properties are often used.

chenglou commented 7 years ago

@aditya2272sharma We were discussing on making working with data- and aria- better (just like we did with style), not eliminating them...

aditya2272sharma commented 7 years ago

appreciate your efforts, apologies for the previous comment.

ha404 commented 7 years ago

I'd love to practice this if and when dataset is implemented: https://statusok200.wordpress.com/2017/01/10/dataset-in-react/.

Would this be the recommended method against binding or is there a more performant way to do this?

Thanks!

xyzdata commented 7 years ago

👍 not too bad!

https://github.com/gildata/RAIO/issues/174

kumavis commented 6 years ago

We use data-* as standard way for tools to associate data with an element. Specifically we use a utility that saves + restores form data in form elements that are labeled with a key in their dataset.

While this problem could be solved entirely using react-based tools, react is not the standard interface, the DOM is the standard.

Here is my +1 for dataset support

t1ger-0527 commented 6 years ago

If anyone still trying to pass an object dataset to a component, this gist might help. https://gist.github.com/t1ger-0527/f839a3f59bdcad3300e3e4865bb09027

leidegre commented 6 years ago

@gaearon @syranide how would you deal with event handlers in lists without the data-* attribute?

For example,

import React from "react";

class List extends React.Component {
  constructor() {
    super();

    this.state = { list: ["a", "b", "c"] };

    this._onChange = e => {
      this.setState(prevState => {
        prevState.list[parseInt(e.target.dataset.index)] = e.target.value; // you get the idea...
      });
    };
  }

  render() {
    return (
      <div>
        <ul>
          {this.state.list.map((value, index) => (
            <li key={index}>
              <input
                data-index={index}
                value={value}
                onChange={this._onChange}
              />
            </li>
          ))}
        </ul>
      </div>
    );
  }
}

The only alternative solution that I know to this problem is to use arrow functions/function closure to bind the index to a create specific item event handler. BUT doesn't that create a new function for every render? So, if I had a lot of rows, wouldn't that generate a lot of GC garbage?

markerikson commented 6 years ago

@leidegre : yes, that would be the preferred approach, and yes, it does create functions in every re-render. Please see React, Inline Functions, and Performance for a discussion of that.

If performance is a real concern for your situation, you can try memoizing your rendering so that you don't have to re-create the bound functions every time you re-render, as well as virtualizing the list so you aren't actually putting all the rows in the DOM.

omriLugasi commented 5 years ago

@markerikson This is an issue that should be resolve by adding option to return an object without render every time the component ( and without render every time an arrow function). you right that there is an option to minimize it by virtualised the grid or list. For me this is a big issue, i want to write react in the best way but react not let me the option to write in the best way

In the example above i only want to know on each item in the list you clicked, but i don't want to re render every time the function......

So sorry your answer is not enough for me, i still need to work hard to know each item clicked on the list....

for example you can see @leidegre example

ghost commented 5 years ago

@markerikson I don't know who gathered statistics on the preferred approach, but my preferred approach is to use the data- attributes rather than doing extra work away from the call-site to memoize a simple event handler.

The React docs on this subject give zero reasoning for their opinion.

i5ar commented 5 years ago

Something like a dataSet property will make code much more readable for those that can't use Node.js and JSX.

const e = React.createElement;

e(
  "span", {
  id: "foo",
  className: "bar",
  dataSet: {foo: "bar"},
  style: {color: "red"}
  }
);

Otherwise, you have to wrap data attributes with quotes (which is awful):

const e = React.createElement;

e(
  "span", {
  id: "foo",
  className: "bar",
  "data-foo": "bar",
  style: {color: "red"}
  }
);
mctrafik commented 2 years ago

How is this still open in 2022??!?? For something that's universally supported by every browser: https://caniuse.com/dataset yet React doesn't let you just use it.

mctrafik commented 1 year ago

Polite ping. Still checking in on this... and would like a resolution.

hs-sean-grant commented 1 year ago

The implementation of dataset differs on the browser. Some actually will propagate changes into the data attribute while others do not. This becomes a problem when some libraries reference data attributes while others reference dataset. If such an idea is implemented, it should avoid writing into dataset and instead follow the suggestion in OP to transform the values into data attributes (to be read by dataset).

hs-sean-grant commented 1 year ago

https://gist.github.com/t1ger-0527/f839a3f59bdcad3300e3e4865bb09027

This could be put into a recursive decorator to automatically apply to all children within your project.

mctrafik commented 1 year ago

@sean-a-grant you're wrong. The dataset support is pretty much universal except for really-really-really old browsers. See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset and https://caniuse.com/?search=dataset

React should support this API as it's ubiquitous across all browsers. The existing workarounds require weird hacks like direct DOM manipulation or writing out data-* attributes which isn't desided!

On a meta note: it feels like there's a lot of nub developers who don't know element.dataset is for... and that's fine, but they really ought to not be commenting.

hs-sean-grant commented 1 year ago

totally got this mixed up with jquery's "data" my b. +1 on wish to support dataset for the OPs reasoning, would be way cleaner to implement.