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

refactor(react-jsx): solve some code smell #21826

Closed
wants to merge 1 commit into from

Conversation

liuarui
Copy link

@liuarui liuarui commented Jul 8, 2021

Summary

When I learn the react source, I found some small errors that don't affect code execution.

Refactor some react/jsx ReactJSXElement.js code

Test Plan

just refactor, and I have passed the local test

@liuarui
Copy link
Author

liuarui commented Jul 8, 2021

It seems that Circleci has a problem, I am trying to restart Check

@sizebot
Copy link

sizebot commented Jul 8, 2021

Comparing: 241485a...3dd171a

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 = 127.20 kB 127.20 kB = 40.76 kB 40.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 130.01 kB 130.01 kB = 41.67 kB 41.67 kB
facebook-www/ReactDOM-prod.classic.js = 404.75 kB 404.75 kB = 74.83 kB 74.83 kB
facebook-www/ReactDOM-prod.modern.js = 393.15 kB 393.15 kB = 73.07 kB 73.07 kB
facebook-www/ReactDOMForked-prod.classic.js = 404.75 kB 404.75 kB = 74.83 kB 74.83 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 3dd171a

1. reduce `getComponentNameFromType` call

2. reduce `let` use
@liuarui
Copy link
Author

liuarui commented Jul 8, 2021

It seems that Circleci has a problem🤣

let didWarnAboutStringRefs;
let specialPropKeyWarningShown,
specialPropRefWarningShown,
didWarnAboutStringRefs;
Copy link
Contributor

@bvaughn bvaughn Jul 8, 2021

Choose a reason for hiding this comment

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

This change seems purely subjective. Some may prefer one way, others another. We usually don't merge subjective changes to the code. 😄

Copy link
Author

Choose a reason for hiding this comment

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

get, thank you for your reply.
I am sorry to bother your time, my intention just wants the React code better.🤣

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries. It's okay!

Copy link
Author

Choose a reason for hiding this comment

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

hahaha, But another change I hope that can make changes in subsequent, because it does make a function call without necessary.

@liuarui
Copy link
Author

liuarui commented Jul 8, 2021

i will close

@liuarui liuarui closed this Jul 8, 2021
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