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

Consider using textContent rather than innerText #1582

Closed
davidroeca opened this issue Dec 14, 2022 · 3 comments
Closed

Consider using textContent rather than innerText #1582

davidroeca opened this issue Dec 14, 2022 · 3 comments

Comments

@davidroeca
Copy link

davidroeca commented Dec 14, 2022

Is there a rationale for setting innerText rather than textContent in the setData function?

They do behave a bit differently -- I think textContent allows for more whitespace in some scenarios. However, it also appears to be faster than innerText which may be a nice added bonus to an already fast spreadsheet library.

Context: I had to dig pretty deep to understand why my tests were failing -- they rely on jsdom which is the go-to headless engine for jest. Turns out that jsdom doesn't support innerText due to its reliance on a layout engine. So for me, mocking innerText's setter and getter to be textContent helped make my tests pass. The workaround mentioned in the jsdom issue solved my problems, so I'd call this a suggestion rather than a request. But it could help users who write react applications and end up using a testing suite such as Jest that relies on jsdom.

@hodeware
Copy link
Collaborator

hodeware commented Jan 2, 2023

That is a good suggestion and we will implement that as as soon as possible.

@hodeware
Copy link
Collaborator

v4.11.0+ implement those changes. We are adopting some tests using jsdom. If you have something to share it would be much appreciated.

@davidroeca
Copy link
Author

@hodeware here's what I used for innerText. There are still issues interacting with the table, but this works for reading what's in the table

// Taken from below (with a setter as well):
// https://github.com/jsdom/jsdom/issues/1245#issuecomment-445848341
// This is required for jspreadsheet tests to work appropriately, since
// jspreadsheet's createCell function updates innerText
Object.defineProperty(global.Element.prototype, 'innerText', {
  get() {
    return this.textContent
  },
  set(value) {
    this.textContent = value
  },
  configurable: true,
})

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

No branches or pull requests

2 participants