-
Notifications
You must be signed in to change notification settings - Fork 47.4k
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
Fix missing else in ReactDOMServerFormatConfig.js #22431
Conversation
Comparing: d9fb383...3c7d297 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
eb3baf7
to
391e2d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should be
//children is always a string
@akgupta0777 fixed the comment and solved conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested a few changes. Also, I'd suggest rebasing and squashing this into a single commit, because merge commits are discouraged in PRs. There are many ways to do this. Here's one way:
git checkout fix-22309
# rewind your branch to before your first commit
# If you've made more commits, replace "4" with the number of commits in your PR
git reset HEAD~4
# stash all changes in those commits
git stash --include-untracked
# rebase this branch with the latest changes from upstream
git remote add upstream https://github.com/facebook/react.git
git pull upstream main
# stage your changes to the one file you changed
git checkout stash@{0} -- packages/react-dom/src/server/ReactDOMServerFormatConfig.js
# undo the checkout, leaving the changed file in your working directory
git reset
#
# NOW, MAKE YOUR ADDITIONAL EDITS FROM CODE REVIEW AND SAVE THEM
#
# commit changes you made
git add packages/react-dom/src/server/ReactDOMServerFormatConfig.js
git commit -m "remove unnecessary code"
# finally, force-push to your PR branch which will clean up history
git push -f
874f7f2
to
2ab364d
Compare
Removing unnecessary code.
2ab364d
to
3c7d297
Compare
@justingrant all fixes are done, please review once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks
What is the procedure to merge this PR? @justingrant |
One of the maintainers will have to review it. There's 200+ open PRs so it might be a while. But this is a narrowly-scoped PR so it's unlikely to get stale soon. Someone should review & merge it sooner or later. |
@crypt096 are you helping in maintaining reactjs? |
@bvaughn please review this PR. |
@bvaughn any updates on this? |
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
Removed else statement from ReactDOMServerFormatConfig.js
Else statement was redundant since value is set anyways.
Solves bug 22309.
Summary
This PR removes redundant code from the ReactDOMServerFormatConfig.js file.
As discussed here 22309
How did you test this change?
Tested it using
yarn prettier
yarn linc
yarn build and test