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

Remove Stack (part 4: remove Stack-only branches and types) #10798

Merged
merged 3 commits into from Oct 2, 2017

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Sep 24, 2017

Last part extracted from #10517.
The first two commits remove unnecessary injections.

The last one is somewhat extensive and removes dead files and all branching on typeof x.tag === 'number'. It also removes precaching logic from the component tree (Stack only). validateDOMNesting takes one less argument because Fiber always passed null instead of childInstance.

For tests, it fixes up ResponderEventPlugin-test.js since it depended on Stack internal names.

@gaearon gaearon force-pushed the remove-stack-again-404 branch 2 times, most recently from 6597323 to ec31847 Compare September 27, 2017 16:30
@gaearon gaearon changed the title Remove Stack (part 4, unsafe: remove Stack-only branches and types) Remove Stack (part 4: remove Stack-only branches and types) Sep 29, 2017
@reactjs-bot
Copy link

reactjs-bot commented Sep 29, 2017

Deploy preview ready!

Built with commit 0a67baa

https://deploy-preview-10798--reactjs.netlify.com

@trueadm trueadm self-requested a review October 2, 2017 10:40
Copy link
Contributor

@trueadm trueadm left a comment

Choose a reason for hiding this comment

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

This is a bit of a harder diff to review as there are so many changes. Maybe we can merge this and test with a sync? Otherwise, it builds locally for me and works, so I'll approve.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 2, 2017

I prepared a sync (see my last diff) and went through the test plan. Worked fine for me.

@trueadm
Copy link
Contributor

trueadm commented Oct 2, 2017

@gaearon Awesome! All good on my side then – great work removing Stack :)

@gaearon gaearon merged commit 5c13e2e into facebook:master Oct 2, 2017
@aweary aweary mentioned this pull request Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants