Fix TypeError dom is null in updateHighlight

[bz2]

Regression introduced by #244 which checks nodeType of a maybe null value.

Basic fix is to check the null, but introduce a little more typing demonstrating how errors like this can be avoided. One new function isTextNode is a narrowing type guard. Doing the same thing inline unfortunately is not nearly as neat.

Drive-by unmix tabs and spaces for everyone's sanity.

Traceback from using latest devtools release:

Uncaught (in promise) TypeError: dom is null
    updateHighlight (index):2571
    mount (index):2609
    update (index):2681
    update (index):2668
    update (index):2722
    createCommit (index):2773
    onCommit (index):2962

And just a general observation, the code has a lot of telling the type checker to shut up with any casts. It looks like importing at least VNode from preact/internal would make life easier, worth trying as a change? It has the HTMLElement | Text | null annotation on _dom for instance.

[changeset-bot]

💥 No Changeset

Latest commit: 3fcf1d9

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[marvinhagemeister]

Do you remember what the component looks like that triggered this error? I'd love to make sure that we don't regress on this in the future.

And just a general observation, the code has a lot of telling the type checker to shut up with any casts. It looks like importing at least VNode from preact/internal would make life easier, worth trying as a change? It has the HTMLElement | Text | null annotation on _dom for instance.

Agree, there is always room for improvement in any code bases. Happy to review any PRs you have in store 👍

[bz2]

Would be good to add a test case, unfortunately the code I'm using involves hooks, suspense, async, and all kinds of wonderful complications so I'm not sure the best way to work out what the broken state is. Actually get two different variations. One fails under setState in a promise callback in a provider which is early on so the whole page breaks (remains <Skeleton />). The other fails under setState under a custom use... method, but the whole page is complete and renders. Suggestions welcome!

I've got another commit which goes a bit further on typing up the internals here which will propose separately.

Any idea why the build gating doesn't seem to have run?

[marvinhagemeister]

Thank you so much for the PR 🙌 Will cut a new release soonish 👍

[marvinhagemeister]

Not sure what's up with the CI. DevOps stuff is admittedly not my strong suite :S