Skip to content

Conversation

@syranide
Copy link
Contributor

What the title says.

  • Test plan: grunt test

@sophiebits
Copy link
Collaborator

Somewhere else we use .textContent = '' to clear… maybe that's faster?

@syranide
Copy link
Contributor Author

@spicyj I distinctly remember that people have been testing this since forever and using the approach React already uses is the faster. But I'll test. That's funny, it's not even using the fastest one from the test...

@syranide
Copy link
Contributor Author

@spicyj I can't find that anywhere. Perhaps you're thinking of this.updateTextContent('');?

@sophiebits
Copy link
Collaborator

Yeah, I guess that's what I was thinking of.

@syranide
Copy link
Contributor Author

syranide commented Feb 3, 2015

@zpao This seems like a straight-forward refactor to me, agreed? :)

EDIT: Hmm, it went from 3 to 2 locations because of a recent PR, perhaps not as meaningful anymore?

@zpao
Copy link
Member

zpao commented Feb 3, 2015

Ok sure. Let's make lint pass though :) I think this is a special case of assignment so just disable the rule in the file (there were a couple others I did this for in my eslint PR).

@syranide
Copy link
Contributor Author

syranide commented Feb 4, 2015

@zpao Ah, grunt lint is somewhat clogged up with other errors :) anyway, fixed and also updated the license. 👍 ?

@gaearon
Copy link
Collaborator

gaearon commented Mar 26, 2016

We’re trying to get better at reviewing PRs timely but it’s an ongoing process. For now I’m just cleaning up stale PRs that didn’t get merged for one reason or another, and in this case unfortunately it seems like a plain oversight due to it being low priority. Since it’s a tiny change I’m not sure whether you are interested in resubmitting it, but please feel free to. For now, I’m closing as it doesn’t merge cleanly anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants