0

So my code works, but I just wanted clarification whether this is good coding practice or if it will cause problems later on.

As a bit of background this is similar to my previous question How to filter through array of components/elements in reactjs. But this time, instead of filtering an array of dom elements, I'm filtering an array of components. Here's how I did it:

Parent:

delete_this(index)
{   
    let key = index._reactInternalInstance._currentElement.key;
    this.repeats = this.repeats.filter( (item) =>  
    {   
        return item.key !== key;
    }); 
    this.setState({ repeats: this.repeats });  
}

Child: 

delete_this(value)
{   
    this.props.delete_this(value);
}  
render()
{
    <button onClick={this.delete_this.bind(this, this)} ref={ (input) => { this.button = input; } }>delete</button>
}

I tried doing a filter on the object itself but it didn't work so I used the key instead.

Community
  • 1
  • 1
A. L
  • 11,695
  • 23
  • 85
  • 163

1 Answers1

2

As mentioned in your other question that is very similar to this, you should not be relying on internal property like _reactInternalInstance.

They're "private" and the React team can technically deprecate it at any time. I don't know the React teams policy on semver, but I highly doubt changes to an internal api count as a breaking change.

So to answer your question, yes it will possibly cause issues down the line.

You can simply pass in the id to the delete method directly:

<button onClick={() => this.props.delete_this(this.props.id)}>delete</button>
Chris
  • 54,599
  • 30
  • 149
  • 186
  • Yeah, at this point, I think it's probably just the same as passing in an id – A. L Jan 04 '17 at 01:46
  • It's actually quite different :) `key` is one of very few "special" attributes in React (unlike Angular which has a bunch of ng-this ng-that everywhere). From the 4 or 5 similar questions you have posted, you seem like you want to avoid the concept of ids or similar. I understand at the start you want to avoid added complexity, but nearly every app anything repeated will have a concept of ids (which is local to your app, but makes the repeated item unique), so you might as well use them. I really encourage you to go back and look at that codepen I posted in your other question – Chris Jan 04 '17 at 01:56
  • I'm not trying to hassle this point, but the map isn't the important part - its the way of thinking about state, and how it should flow down into the component. Regardless good luck – Chris Jan 04 '17 at 03:54
  • I just find that an infinitely increasing ID number to be somewhat odd/annoying, which is why I was just preferring to delete stuff by matching elements. But I'll use what's safer. – A. L Jan 04 '17 at 03:57
  • You don't actually have to self manage unique ids. As mentioned in the other answer, the example of unique id was very primitive. Most times you will be interacting with some persistant storage layer (such as a database) where you will simply be using the id provided by that. In which case all of this discussion (across both questions) becomes instantly a moot point – Chris Jan 04 '17 at 04:00
  • For that part, the component generation is user side, and they can generate as many of the subcomponents as they need as to fill their required information. – A. L Jan 04 '17 at 04:08
  • Which will surely eventually end up in said persistence layer? – Chris Jan 04 '17 at 04:10
  • It will end up in the database eventually, yes. I'll start using the ID system once I get back to it. – A. L Jan 04 '17 at 04:14