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

DX improvements #867

Open
iamstarkov opened this issue Feb 19, 2018 · 9 comments
Open

DX improvements #867

iamstarkov opened this issue Feb 19, 2018 · 9 comments

Comments

@iamstarkov
Copy link

Description

As a react ecosystem developer, i want to ensure my HoCs work with RHL. Right now it is hard to achieve.

Actual behavior

Nothing helps react ecosystem developer to verify that RHL works with HoCs they create.

Expected behavior

I would like to see some few improvements:

  • in-depth walk through RHL's life-cycle methods to understand how does it work in details
  • sort of SDK to mimic different RHL's life-cycle methods to ensure HoCs works with it the way developers intend to (perhaps RHL-dev).
  • few recipes of SDK usage with different popular libraries and test runners (react-redux, react-intl, etc and mocha, ava, etc)
@theKashey
Copy link
Collaborator

theKashey commented Feb 19, 2018

Use React-Hot-Loader version 4, RC was shipped yesterday.
And it just works.

I mean we literally dont know a method, a way to make HoC/decorator/component NOT to work with a new version.

@iamstarkov
Copy link
Author

@theKashey the claim "it just works" was in the docs before. it didnt "just work". Trust me, i would like it to (cssinjs/react-jss#117).

the problem is still in debugging it. How did you verify that it works with any possible HoC/component/etc? can it be put in words? can some RHL/dev file be created with integration/test helpers in order to test components against RHL and verify it would works with them?

another situation. What if RHL doesnt work with my obscure HoC (it will eventually happens)? Or if RHL does work, but not in a way i would expect it to?

about the detailed documentation:

  • it will help with onboarding to the project for newcomers.
  • thus it will help community developers to know how it works and help out with pull-requests.

about rhl/dev or some kind of integration/test helpers:

  • it will allow RHL to create "Canary in the Goldmine" for approach for continuous releases to ensure top (3/5/10/N) HoCs didnt break with the new pull-request.
  • community developers will be able to reduce cognitive load and stress, because with those integration/test helper they will be able to ensure their HoCs work.

@gaearon
Copy link
Owner

gaearon commented Feb 20, 2018

My two cents: I'm not actively involved with the project anymore but I think we might need to fundamentally rethink how it works with the assumption that we can change something in React if necessary. Both the way we proxy classes and the way we traverse the rendered tree seem potentially flaky to me. I'm sorry I can't spend more time on diving into details but I would love to see a proposal in React RFCs that suggests hooks necessary on React side to remove all the hacky code from RHL and make it work 100% consistently.

@gregberge
Copy link
Collaborator

@gaearon I agree, now that the project is stable, we have to make it cleaner, and to do it we have to make changes in React.

@gaearon
Copy link
Owner

gaearon commented Feb 20, 2018

Does the project currently add any custom hooks to components beyond the React API? I noticed something called componentDidRender and wasn't sure what that is.

@theKashey
Copy link
Collaborator

theKashey commented Feb 20, 2018

Currently, we have one "problem", one "feature", and one "thing to improve".

  1. The problem - we wrap all SFC into classes to have an instance to handle during the tree traversal.
    Not actually we need it, but we need props and context, to render a component, and even if we have the props we need - we don't a have context.
  2. The feature - all SFC we wrap, are "IndeterminateComponents", ie they are returning an instance(Used in Relay, react-stand-in incorrectly picks up Relay containers as functional components #775). As result we got Compat mode.  #833 - some other tree-traversals could fail. Remove the point 1, and you can remove the point 2. IndeterminateComponents could be returned only from SFC, and we just killed them.
  3. Some stuff requires componentWillReceiveProps to sync-update tree before the render. Thus my lead to renderx3 - hot-render, render, force-update after timeout.

Meanwhile - all this stuff is required to fake element.type and hot-render the tree, and will not be required, if element type comparison, somewhere in the depths of react-dom, will be controllable.
Ie no wrapping SFC by classes to property render them, no tree-traversal. Only fuzzy comparing and hacking the instances, injecting the updated code.

Anyway - I've promised at least trice to document how v4 works. It's time to fulfil the promises.

@theKashey
Copy link
Collaborator

I've started adding some pages into Wiki. The first one - How-React-Hot-Loader-works

@theKashey
Copy link
Collaborator

Just after writing down how it works, I've point a few moments which may no longer work as I expect, or should work differently.

@iamstarkov
Copy link
Author

@theKashey thanks for this

@gregberge gregberge removed the v4 label Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants