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

Cannot read property 'return' of null #12449

Closed
jquense opened this issue Mar 24, 2018 · 10 comments
Closed

Cannot read property 'return' of null #12449

jquense opened this issue Mar 24, 2018 · 10 comments
Assignees

Comments

@jquense
Copy link
Contributor

jquense commented Mar 24, 2018

Running into a strange issue that i cannot reproduce, it happens when running tests in Karma using 16.3.0-alpha.3

image

The issue is here:

if (nextUnitOfWork === null) {
// This is a fatal error.
didFatal = true;
onUncaughtError(thrownValue);
break;
}
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
const failedUnitOfWork = nextUnitOfWork;
replayUnitOfWork(failedUnitOfWork, isAsync);
}
const sourceFiber: Fiber = nextUnitOfWork;
let returnFiber = sourceFiber.return;

My test is throwing an error because of a missing onChange handler when rendering a checkbox with checked but no handler. What appears to be happening is that nextUnitOfWork is not null and misses the first branch, replayUnitOfWork is run, after which nextUnitOfWork is null and sourceFiber doesn't exist and so access of return fails.

I've tried for a while to reproduce it, the following codesandbox is a simplified version of the code that is throwing, but the error isn't present there :/ https://codesandbox.io/s/0332y243zp

@acdlite
Copy link
Collaborator

acdlite commented Mar 24, 2018

Hmm, I was concerned this might happen. We assume that replayUnitOfWork will be idempotent but looks like you've found a case where it's not. I'll take a look at this.

@acdlite acdlite self-assigned this Mar 24, 2018
@acdlite
Copy link
Collaborator

acdlite commented Mar 24, 2018

Changing line 877 to

const sourceFiber: Fiber = failedUnitOfWork;

isn't a sufficient fix because if it's not equal to nextUnitOfWork, that means the stack is misaligned.

One option is to add an invariant that throws if nextUnitOfWork !== failedUnitOfWork. Usually we avoid DEV-only invariants, but since this should only be possible in an error case, anyway, maybe we can get away with it.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

Repro case from #12497: https://jsfiddle.net/thysultan/Luktwrdm/264/

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

Btw, there's another potentially related bug that happens if you remove this feature flag override that manifests as an Unexpected pop warning. Maybe we should change this test suite to use the feature flag that people use in real bundles? This doesn't leave me very confident.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

Added failing tests in #12508.

@gaearon gaearon mentioned this issue Apr 1, 2018
3 tasks
@gaearon
Copy link
Collaborator

gaearon commented Apr 3, 2018

Fixed in #12508

@gaearon gaearon closed this as completed Apr 3, 2018
@onpaws
Copy link

onpaws commented Apr 5, 2018

Cheers, this PR just saved us from what would have been a confusing day!

@JeffUberman
Copy link

I've upgraded to React v16.3.2 and am seeing a similar error:

screen shot 2018-04-19 at 3 02 50 pm

It's referencing a similar area of the React codebase, without a null check:

screen shot 2018-04-19 at 3 06 20 pm

@gaearon
Copy link
Collaborator

gaearon commented Apr 19, 2018

Please file a new issue with a reproducing case. Thanks.

@facebook facebook locked as resolved and limited conversation to collaborators May 4, 2018
@gaearon
Copy link
Collaborator

gaearon commented May 4, 2018

To everyone who "+1"s the previous comment: sorry, but this doesn't help us solve the issue at all.

If you're seeing this after upgrading both react and react-dom to 16.3.2 or higher, please file a new issue with a minimal reproducing case extracted from your project. This will be very helpful to everyone, and we'll be able to quickly diagnose and fix this.

plondon pushed a commit to blockchain/blockchain-wallet-v4-frontend that referenced this issue May 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants