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

[Fizz] Let value override defaultValue if both are specified #21369

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

sebmarkbage
Copy link
Collaborator

There's a DEV warning for this case but we still test for the production behavior.

It's a bit annoying since it might be more efficient to just rely on the loop and iteration order. I was almost ready to die on this hill but not quite.

There's a DEV warning for this case but we still test for the production
behavior.
@facebook-github-bot facebook-github-bot added React Core Team Opened by a member of the React Core Team CLA Signed labels Apr 27, 2021
break;
default:
pushAttribute(target, responseState, propKey, propValue);
break;
}
}
}

if (checked !== null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why this instead of props.checked? Well because we have to do the loop and check every property name anyway. Where as props.checked likely wouldn't be fast pathed since the hidden class of these props are often different. So it would be an unnecessary map look up. However, I do a props.id check below but that's just because I expect that code path to go away.

@sizebot
Copy link

sizebot commented Apr 27, 2021

Comparing: 9a25916...379f853

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 122.76 kB 122.76 kB = 39.42 kB 39.42 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.34 kB 129.34 kB = 41.51 kB 41.51 kB
facebook-www/ReactDOM-prod.classic.js = 406.92 kB 406.92 kB = 75.31 kB 75.31 kB
facebook-www/ReactDOM-prod.modern.js = 394.95 kB 394.95 kB = 73.41 kB 73.41 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.92 kB 406.92 kB = 75.31 kB 75.32 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMServer-prod.modern.js +1.48% 71.74 kB 72.80 kB +0.95% 14.82 kB 14.96 kB
oss-experimental/react-dom/umd/react-dom-unstable-fizz.browser.production.min.js +0.57% 32.21 kB 32.40 kB +0.59% 10.89 kB 10.96 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.browser.production.min.js +0.56% 32.04 kB 32.22 kB +0.51% 10.78 kB 10.84 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.node.production.min.js +0.56% 32.34 kB 32.52 kB +0.52% 10.81 kB 10.86 kB

Generated by 🚫 dangerJS against 379f853

@sebmarkbage sebmarkbage merged commit 2182563 into facebook:master Apr 27, 2021
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
There's a DEV warning for this case but we still test for the production
behavior.
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.

4 participants