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

Fix a mistake in ReactChildren refactor #18380

Merged
merged 6 commits into from
Mar 25, 2020

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 24, 2020

Of course I broke something in #18332. Luckily internal tests caught it.

"there's probably at least not many bugs." — @sebmarkbage

I changed signature of one of the internal functions, but forgot to adjust the callsite. So there was a missing argument. The fix for that is 20fc0bd.

To prevent this from happening again and to turn my mistake into a learning experience, I added:

  1. Regression test for map returning an array — we didn't have that. 344e56c
  2. Flow types. f66dc55

This fixes the bug.
These aren't directly observable. The arg wasn't used, it's accidental and I forgot to remove. The retval was triggering a codepath that was unnecessary (pushing to array) so I removed that too.
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f66dc55:

Sandbox Source
upbeat-paper-ulexy Configuration

@sizebot
Copy link

sizebot commented Mar 24, 2020

Warnings
⚠️ Failed to fetch build artifacts for base commit: 2ba43ed

Size changes (experimental)

Generated by 🚫 dangerJS against e818f10

@sizebot
Copy link

sizebot commented Mar 24, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against e818f10

@@ -111,16 +121,18 @@ function mapIntoArray(children, array, escapedPrefix, nameSoFar, callback) {
if (childKey != null) {
escapedChildKey = escapeUserProvidedKey(childKey) + '/';
}
mapIntoArray(mappedChild, array, escapedChildKey, c => c);
mapIntoArray(mappedChild, array, escapedChildKey, '', c => c);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the fix. Oops.

@sebmarkbage
Copy link
Collaborator

lol at that branch name.

@gaearon gaearon merged commit 6498f62 into facebook:master Mar 25, 2020
@gaearon gaearon deleted the kids-arent-alright branch March 25, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants