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

React / ReactDOM split proposal #5974

Closed
iamdustan opened this issue Feb 3, 2016 · 5 comments
Closed

React / ReactDOM split proposal #5974

iamdustan opened this issue Feb 3, 2016 · 5 comments

Comments

@iamdustan
Copy link
Contributor

Following up on #5925 with a bit more planning.

From what I understand ReactDOM cannot be split completely from React until there is a shared global owner. This is referenced in @sebmarkbage’s gist: https://gist.github.com/sebmarkbage/2d6c52dd9d2e55e9d139.

The relevant part:

"owner" - The "owner" concept relies on being able to track a shared
global state flag at the time of creating elements. This means that we have to
create a dependency into a single shared global stateful module for all React
components. It also means that effectively no React render function is truly a
pure function since creating an element depends on global state. It is not
idempotent. That means that you can't reliably use things like memoized
functions and all kind of other functional goodies.

React Native currently appears to have a migration path in the following two files (from c29642d):


Any solutions need to have a way to share the ReactCurrentOwner module.

  • RN should get this for free because of @providesModule (if I understand how
    that works correctly). In the future the same npm solution for ReactDOM will
    apply to RN.
  • React + ReactDOM (npm distro): a shared npm module react-current-owner will
    accomplish the shared dependency.
  • React + ReactDOM (browser distro): this will likely need a similar hack to the
    current React.__SECRET_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.

The first two should be pretty straightforward. Create a new package react-current-owner which is purely the ReactCurrentOwner.js file, split out. Published to 1.0.0 never to be changed. Add this to both React and ReactDOM package.json files. Publish, fin, carry on.

The browser distro will need to have some custom build files. My current idea is that the React browser distro gets a __SECRET_OWNER_DO_NOT_USE_OR_YOU_WILL_BE_FIRED hook that ReactDOM can hook onto. The majority of this is purely a chore to work through.

I am curious if this would provide an attack vector by leaking this internal state. The current ReactDOM hook does not leak internals as far as I’m aware. Although this may not be important since the REACT_DEVTOOLS_HOOK also allows a visitor to a React page to get into internal state.

Before continuing any work on #5925, does this proposal make sense? What alternative solutions has the team considered?

@jimfb
Copy link
Contributor

jimfb commented Feb 3, 2016

Let's keep the discussion in the main thread, unless this is a distinct alternative proposal (this looks to me like a continuation of the same proposal/discussion). Having a single proposal being discussed in multiple separate threads is confusing and harder to track.

@jimfb jimfb closed this as completed Feb 3, 2016
@iamdustan
Copy link
Contributor Author

The initial discussion happened on twitter and that current PR was a 15 minute thing from the twitter convo. I was taking a step back with this issue to ensure that the requirements were clear and the solution was agreed upon before continuing the PR.

@sebmarkbage sebmarkbage reopened this Feb 4, 2016
@jimfb
Copy link
Contributor

jimfb commented Feb 4, 2016

PR closed in favor of discussion here.

@iamdustan iamdustan changed the title React / ReactDOM split. Discussion for #5925 React / ReactDOM split proposal Feb 11, 2016
@iamdustan
Copy link
Contributor Author

Since there hasn’t been any additional feedback I took a smaller step in this direction with #6022.

@iamdustan
Copy link
Contributor Author

This has been handled by the packaging of the reconcilers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants