Skip to content

Conversation

@Mhmdrza
Copy link

@Mhmdrza Mhmdrza commented Feb 13, 2021

typeof vnode.type will throw an Execption if vnode being null. A simple truthy check fixes the issue.

It's a bug fix. related to async-loader.

Did you add tests for your changes? No

Summary
In some circumstances vnode may become null, and trying to access a property on that will rise a TypeError exception. here is a screenshot of the exception:
image

This rare issue happened in a workplace repo, hence I can't provide you the code; but If you guys think It is absolutely necessary to have a look by yourself, I can try to reproduce it in new repository ( I'd actually tried once, but it wasn't successful. )

Does this PR introduce a breaking change?
No breaking changes.

Other information

Please paste the results of preact info here.

Environment Info:
System:
OS: Windows 10 10.0.18362
CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
Binaries:
Node: 14.15.4 - C:\Program Files\nodejs\node.EXE
Yarn: 1.21.1 - D:\Program Files (x86)\Yarn\bin\yarn.CMD
npm: 6.14.10 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 88.0.4324.146
Edge: Spartan (44.18362.449.0)
npmPackages:
preact: ^10.3.2 => 10.5.7
preact-cli: ^3.0.0 => 3.0.3
preact-render-to-string: ^5.1.4 => 5.1.12
preact-router: ^3.2.1 => 3.2.1

typeof vnode.type will throw an Execption if vnode being `null`. A simple truthy check fixes the issue.
@Mhmdrza Mhmdrza requested a review from a team as a code owner February 13, 2021 00:45
@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2021

⚠️ No Changeset found

Latest commit: f4f5dd9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

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.

1 participant