Skip to content

Conversation

@koddsson
Copy link
Contributor

textContent seems like a better alternative and we seem to favor that already so if nothing else we can be consistent 😸

$ git grep innerText | wc -l
      30
$ git grep textContent | wc -l
     235

There are some slight differences between the two and there is probably some code that relies on those differences but there aren't many instances so I can manually check over those to make sure nothing is breaking.

Reading 📚

@koddsson koddsson requested review from a team and dgraham January 26, 2018 16:44
node: node.property,
message: 'Prefer textContent to innerText',
fix(fixer) {
return fixer.replaceText(node.property, 'textContent')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can run this on a codebase with --fix and it will fix it! ✨

type: 'Identifier'
}
],
output: 'document.querySelector("js-flash-text").textContent = "bar"'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will assert the output if this rule was run with --fix.

Copy link
Contributor

@dgraham dgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As innerText is aware of CSS styling, it will trigger a reflow, whereas textContent will not.

As good a reason as any to prefer textContent.

Copy link
Contributor

@keithamus keithamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🆒

@koddsson
Copy link
Contributor Author

koddsson commented Jan 29, 2018

I want to merge and publish this but I'm not sure if I have the rights on npm to do that. @josh are you the one that publishes?

@josh
Copy link
Contributor

josh commented Jan 29, 2018

I want to merge and publish this but I'm not sure if I have the rights on npm to do that.

I just added you to our @github org on npm.

@koddsson koddsson merged commit ab7ac24 into github:master Jan 29, 2018
@koddsson koddsson deleted the add-no-inner-text branch January 29, 2018 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants