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

Invalid value for data- prop. Expected behavior? #12614

Closed
a-laughlin opened this issue Apr 14, 2018 · 4 comments
Closed

Invalid value for data- prop. Expected behavior? #12614

a-laughlin opened this issue Apr 14, 2018 · 4 comments

Comments

@a-laughlin
Copy link

Per this blog post, react allows passing data attributes freely. In React 16.2.0, on MacOS 10.13 Chrome 65, freely seems to stop short of functions.

Replication steps:

ReactDOM.render(
  React.createElement('h1',{['data-foo']:x=>x}),
  document.getElementById('root')
);

image

If the behavior is expected (or not), links/recommendations on how to silence or disable it are welcome. As console errors with stack traces printed, three of those messages scroll a whole page of the console on every reload. I'd love to find a way around it that doesn't involve hacking the app code or React just to disable a warning. Following this closed thread about console warn vs error led to a currently open thread for warning output hooks. I'm currently trying negative lookaround regex filters in chrome dev tools, and those are locking up the dev console process.

edit: A dev tools filter of -Invalid solved the issue well enough for me. Leaving this post in case it is a bug.

@gaearon
Copy link
Collaborator

gaearon commented Apr 14, 2018

What are you trying to do? I don’t understand.

React 16 passes string attributes (and some others) through. But there is no meaning attached to passing a function to a data attribute. It would get stringified even if React allowed it.

Can you explain the purpose of this code in practice?

@a-laughlin
Copy link
Author

a-laughlin commented Apr 17, 2018

Hi Dan,

A brief backstory before jumping into the in-practice examples below. I've been exploring patterns in React apps with insufficient benefits to justify their maintainability costs vs. other patterns. One such pattern was stateless functional components vs. class components. SFCs' reduced surface area creates fewer opportunities for rigid couplings and dependency graphs, and are generally easier to read. Eventually, I started questioning SFCs too. N SFCs adds N interfaces to the app. Each interface has to be learned, and each interface creates opportunities to couple elements <-> handlers <-> function scopes <-> data structures. I eventually concluded that almost no cases did the benefits of per-component interfaces outweigh their costs. String components have fewer tradeoffs. A string like 'div' adds no abstractions to learn, no coupling surface, and no vectors for unnecessary dependency graphs to grow.

All the base components I write now are string components. Each is wrapped with a composition function. e.g.:

export const toHOCComposer = (str)=>(...fns)=>compose(...fns)(str);
export const [Div,Img,Input] = 'div,img,input'.split(',').map(toHOCComposer);

Custom HOCs make working with them efficient. The resulting components often look like:

// these render to <div><input ... /></div>
export const UserNameInput = Input(
  mapProps({defaultValue:'users[id].name'}),
  pipeChanges(from('targetValue'),toState('users[id].name')),
);
export const UserAttributeList = Div(
  withItems(pipe(from('users'),mapIdsToItemProps( UserNameInput ))),
);
  • from gets props and state data for pipe access.
  • mapProps is a HOC wrapper around from.
  • withItems maps its arguments to children.

All HOCs' added props hit the string component. This usually isn't a problem because I mostly pass string id's so each component can get its own data directly from state.

The problem arises with certain handler HOCs. Take, for example, recompose's withState.

// This component should toggle between `<div>hello</div>` and `<div>world</div>` on click.
export const HelloWorldDiv = Div(
  withState('msg',`update-msg`,'hello'),
  withItems(from('msg')),
  pipeClicks(cycleArgs('hello','world'),toPropFn(`update-msg`)),
)

Instead of toggling between elements, it throws an error about naming a prop update-msg.

Renaming the prop to data-update-message gets the handler working. As you said, the function gets stringified when it hits the element props. That doesn't matter in this case because React's internal representation of the props holds the function for other HOCs to access. React just throws a warning when the function hits the element.

I've been working on my first public-facing app using this this structure extensively (let me know if you'd like a link to the working code once it's up). The app has a bunch of invalid data prop errors in handler HOC cases like withState. While they don't affect functionality, they're awkward for development, hence posting this issue.

@gaearon
Copy link
Collaborator

gaearon commented Apr 18, 2018

My first thought is I don’t see us changing useful warnings (potentially could mean you forgot to call a method and pass a function by mistake) to accommodate this use case. It seems to me like you’re essentially reimplementing React components but in a much more roundabout way, and that creates difficulties.

@a-laughlin
Copy link
Author

a-laughlin commented Apr 19, 2018

Interesting take on it. I didn't think of it as reimplementing components as much as providing a shorthand for creating string components with more explicit concerns separation. If creating string components is the primary responsibility of a component then yes, it reimplements that responsibility of components while separating other concerns like handler coupling and attribute mapping. The "roundabout" part makes me wonder if I misunderstand what you're saying though, since string>HOCs seems more direct than string>jsx>function/class>HOCs.

Regardless, I agree. Passing string components directly it isn't how most folks use React, so not a case worth supporting.

To summarize learnings, data- attributes' ability to be passed 'freely' only means they will get stringified and added to the DOM, not omitted from the DOM. As a HOC author, there is no way to define a prop like key that will actually be omitted from the DOM, or for HOC users to define a property name like 'ignore-' that allows HOCs to work together without modifying the component or adding a third HOC to filter the prop.

A couple relevant links:
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html
#140

@gaearon gaearon closed this as completed Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants