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

Added ReactFiberStack shared by ReactFiberContext and ReactFiberHostContext #8611

Merged
merged 10 commits into from
Dec 22, 2016
Prev Previous commit
Next Next commit
Tightened up Fiber | null type in ReactFiberStack to always require F…
…iber
  • Loading branch information
Brian Vaughn committed Dec 21, 2016
commit 0b7c5ff1a08826e044d865317bc37ae4f4b63b7f
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ module.exports = function<T, P, I, TI, C, CX>(
// TODO: Only mark this as an update if we have any pending callbacks.
markUpdate(workInProgress);
workInProgress.memoizedProps = workInProgress.pendingProps;
popHostContainer();
popHostContainer(workInProgress);
return null;

// Error cases
Expand Down
22 changes: 12 additions & 10 deletions src/renderers/shared/fiber/ReactFiberHostContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ const {
export type HostContext<C, CX> = {
getHostContext() : CX,
getRootHostContainer() : C,
popHostContainer() : void,
popHostContainer(fiber : Fiber) : void,
popHostContext(fiber : Fiber) : void,
pushHostContainer(container : C) : void,
pushHostContainer(fiber : Fiber, container : C) : void,
pushHostContext(fiber : Fiber) : void,
resetHostContainer() : void,
};
Expand All @@ -53,21 +53,21 @@ module.exports = function<T, P, I, TI, C, CX>(
return rootInstanceStackCursor.current;
}

// TODO (bvaughn) Pass the host container fiber to push()
function pushHostContainer(nextRootInstance : C) {
function pushHostContainer(fiber : Fiber, nextRootInstance : C) {
// Push current root instance onto the stack;
// This allows us to reset root when portals are popped.
push(rootInstanceStackCursor, nextRootInstance, null);
push(rootInstanceStackCursor, nextRootInstance, fiber);

const nextRootContext = getRootHostContext(nextRootInstance);

push(contextStackCursor, nextRootContext, null);
// TODO (bvaughn) Push context-providing Fiber with its own cursor
push(contextStackCursor, nextRootContext, fiber);
}

// TODO (bvaughn) Pass the host container fiber to pop()
function popHostContainer() {
pop(contextStackCursor, null);
pop(rootInstanceStackCursor, null);
function popHostContainer(fiber : Fiber) {
pop(contextStackCursor, fiber);
// TODO (bvaughn) Pop context-providing Fiber with its own cursor
pop(rootInstanceStackCursor, fiber);
}

function getHostContext() : CX {
Expand All @@ -91,6 +91,7 @@ module.exports = function<T, P, I, TI, C, CX>(
return;
}

// TODO (bvaughn) Push context-providing Fiber with its own cursor
push(contextStackCursor, nextContext, fiber);
}

Expand All @@ -99,6 +100,7 @@ module.exports = function<T, P, I, TI, C, CX>(
return;
}

// TODO (bvaughn) Check context-providing Fiber and only pop if it matches
pop(contextStackCursor, fiber);
}

Expand Down
4 changes: 2 additions & 2 deletions src/renderers/shared/fiber/ReactFiberStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ exports.isEmpty = function() : boolean {

exports.pop = function<T>(
cursor : StackCursor<T>,
fiber: Fiber | null, // TODO (bvaughn) Tighten up this type to only accept Fiber
fiber: Fiber,
) : void {
if (index < 0) {
if (__DEV__) {
Expand Down Expand Up @@ -71,7 +71,7 @@ exports.pop = function<T>(
exports.push = function<T>(
cursor : StackCursor<T>,
value : any,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be value : T for a bit stronger guarantees that ensures that we don't push unexpected values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

fiber: Fiber | null, // TODO (bvaughn) Tighten up this type to only accept Fiber
fiber: Fiber,
) : void {
cursor.current = value;

Expand Down