-
Notifications
You must be signed in to change notification settings - Fork 48.8k
Inline some internals, reduce shared/ utilities between isomorphic and renderers #9903
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
Conversation
This breaks IE8. We don't support it.
It was added temporarily to avoid Stack shared state issues across renderers. Not a problem anymore.
It's unlikely we'll deprecate anything else on React.* object soon.
It's only used in one place in isomorphic. It's used more broadly in Stack so we move it there to die.
@@ -1991,10 +1992,6 @@ src/renderers/testing/__tests__/ReactTestRenderer-test.js | |||
* can update text nodes when rendered as root | |||
* can render and update root fragments | |||
|
|||
src/shared/utils/__tests__/KeyEscapeUtils-test.js | |||
* should properly escape and wrap user defined keys | |||
* should properly unescape and unwrap user defined keys |
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 didn't bother porting this test, for isomorphic (which is where this code leaves on) it's only testing flatten
which is not even officially exposed. But I can write it against flatten
specifically if somebody cares.
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.
😍
return '$' + escapedString; | ||
} | ||
|
||
var unescapeInDev = emptyFunction; |
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.
This could just be null
I guess. But doesn't matter since after Stack is deleted we can probably just inline this?
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.
Let's revisit when/if rewrite Children.
See individual commits.
The goal is to narrow down those utilities because they are copied into both bundles.
While we're at it, also making some things DEV-only.
There is a bit more duplication than I'd like, but it's partly due to Stack.
For things like Symbols, I don't think modules are super useful.
Note: this 441a04e breaks IE8 hard.