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.

Sign up for GitHub

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

Jump to bottom

Reduce size by golfing down some pieces 🏌️‍♂️ (-39 B) #1578

Merged
merged 8 commits into from
Apr 30, 2019
Merged

Reduce size by golfing down some pieces 🏌️‍♂️ (-39 B) #1578

merged 8 commits into from
Apr 30, 2019

Conversation

andrewiggins
Copy link
Member

@andrewiggins andrewiggins commented Apr 25, 2019 •
edited
Loading

Summary:
9f6f0fa Move all component diff'ing logic into component diff condition (-6 B)
b547990 Remove dom parameter from diff() (-11 B)
8928b2f Golf createContext (-6 B)
1f7002b Remove unnecessary code from diffChildren (-16 B)
1baa6bb Simplify originalDom check (-4 B) (broke some hydration tests)

Total: -23 B
Total: -43 B
Total: -39 B

@coveralls
Copy link

coveralls commented Apr 25, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling ed15863 on putt-putt into 66f48d8 on master.

marvinhagemeister
marvinhagemeister approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Omg Andre is at it again 👍 💯 I'm always super impressed with your golfs. 🥇

JoviDeCroock
JoviDeCroock approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

I remember being told that the bytes are scared of you, proved it again :D Nice job!

filoozom
filoozom reviewed Apr 26, 2019
View reviewed changes
src/diff/index.js Outdated Show resolved Hide resolved
@jviide jviide mentioned this pull request Apr 26, 2019
Merged
@andrewiggins andrewiggins changed the title Reduce size by golfing down some pieces 🏌️‍♂️ (-23 B) Reduce size by golfing down some pieces 🏌️‍♂️ (-43 B) Apr 26, 2019
@marvinhagemeister
Copy link
Member

The last commit broke two tests regarding hydration behaviour in compat. They test that text nodes are changed on mismatch during hydration. That behaviour is listed in the docs: https://reactjs.org/docs/react-dom.html#hydrate . If we can't find a way around that I'm more than happy to land this PR in master without the last commit :)

@andrewiggins
Copy link
Member Author

I'm looking at it now

@andrewiggins andrewiggins changed the title Reduce size by golfing down some pieces 🏌️‍♂️ (-43 B) Reduce size by golfing down some pieces 🏌️‍♂️ (-39 B) Apr 28, 2019
@andrewiggins
Copy link
Member Author

Reverted the commit and merged in master. Travis should be green now.

@marvinhagemeister
Copy link
Member

FYI: Just rebased the PR against master

@marvinhagemeister marvinhagemeister merged commit 59ee02c into master Apr 30, 2019
@marvinhagemeister marvinhagemeister deleted the putt-putt branch April 30, 2019 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developit developit developit left review comments

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

@filoozom filoozom filoozom left review comments

@JoviDeCroock JoviDeCroock JoviDeCroock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@andrewiggins @coveralls @marvinhagemeister @developit @filoozom @JoviDeCroock