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

🚀 track "principles" compliance #1118

Open
17 of 22 tasks
theKashey opened this issue Dec 9, 2018 · 1 comment
Open
17 of 22 tasks

🚀 track "principles" compliance #1118

theKashey opened this issue Dec 9, 2018 · 1 comment

Comments

@theKashey
Copy link
Collaborator

theKashey commented Dec 9, 2018

https://overreacted.io/my-wishlist-for-hot-reloading/

Correctness

  • (partialy) Hot reloading should be unobservable before the first edit. Until you save a file, the code should behave exactly as it would if hot reloading was disabled. It’s expected that things like fn.toString() don’t match, which is already the case with minification. But it shouldn’t break reasonable application and library logic.
    Hot reload shouldn’t break React rules. Components shouldn’t get their lifecycles called in an unexpected way, accidentally swap state between unrelated trees, or do other non-Reacty things.
  • (partialy)Element type should always match the expected type. Some approaches wrap component types but this can break .type === MyThing. This is a common source of bugs and should not happen.
  • It should be easy to support all React types. lazy, memo, forwardRef — they should all be supported and it shouldn’t be hard to add support for more. Nested variations like memo(memo(...)) should also work. We should always remount when the type shape changes.
    It shouldn’t reimplement a non-trivial chunk of React. It’s hard to keep up with React. If a solution reimplements React it poses problems in longer term as React adds features like Suspense.
  • Re-exports shouldn’t break. If a component re-exports components from other modules (whether own or from node_modules), that shouldn’t cause issues.
    Static fields shouldn’t break. If you define a ProfilePage.onEnter method, you’d expect an importing module to be able to read it. Sometimes libraries rely on this so it’s important that it’s possible to read and write static properties, and for component itself to “see” the same values on itself.
  • It is better to lose local state than to behave incorrectly. If we can’t reliably patch something (for example, a class), it is better to lose its local state than to do a mixed success effort at updating it. The developer will be suspicious anyway and likely force a refresh. We should be intentional about which cases we’re confident we can handle, and discard the rest.
    It is better to lose local state than use an old version. This is a more specific variation of the previous principle. For example, if a class couldn’t be hot reloaded, the code should force a remount for those components with the new version rather than keep rendering a zombie.

Locality

  • (partialy)Editing a module should re-execute as few modules as possible. Side effects during component module initialization are generally discouraged. But the more code you execute, the more likely something will cause a mess when called twice. We’re writing JavaScript, and React components are islands of (relative) purity but even there we don’t have strong guarantees. So if I edit a module, my hot reloading solution should re-execute that module and try to stop there if possible.
  • Editing a component shouldn’t destroy the state of its parents or siblings. Similar to how setState() only affects the tree below, editing a component shouldn’t affect anything above it.
  • Edits to non-React code should propagate upwards. If you edit a file with constants or pure functions that’s imported from several components, those components should update. It is acceptable to lose module state in such files.
  • A runtime error introduced during hot reloading should not propagate. If you make a mistake in one component, it shouldn’t break your whole app. In React, this is usually solved by error boundaries. However, they are too coarse for the countless typos we make while editing. I should be able to make and fix runtime errors while I work on a component without its siblings or parents unmounting. However, errors that don’t happen during hot reload (and are legitimate bugs in my code) should go to the closest error boundary.
  • Preserve own state unless it’s clear the developer doesn’t want to. If you’re just tweaking styles, it’s frustrating for the state to reset on every edit. On the other hand, if you just changed the state shape or the initial state, you’ll often prefer it to reset. By default we should try our best to preserve state. But if it leads to an error during hot reload, this is often a sign some assumption has changed, so we should should reset state and retry rendering in that case. Commenting things out and back in is common so it’s important to handle that gracefully. For example, removing Hooks at the end shouldn’t reset state.
  • Discard state when it’s clear the developer wants to. In some cases we can also proactively detect that the user wants to reset. For example, if the Hook order changed, or if primitive Hooks like useState change their initial state type. We can also offer a lightweight annotation that you can use to force a component to reset on every edit. Such as // ! or some similar convention that’s fast to add and remove while you focus on how component mounts.
    Support updating “fixed” things. If a component is wrapped in memo(), hot reload should still update it. If an effect is called with [], it should still be replaced. Code is like an invisible variable. Previously, I thought it was important to force deep updates below for things like renderRow={this.renderRow}. But in the Hooks world, we rely on closures anyway this seems unnecessary anymore. A different reference should be sufficient.
  • Support multiple components in one file. It is a common pattern that multiple components are defined in the same file. Even if we only keep the state for function components, we want to make sure putting them in one file doesn’t cause them to lose state. Note these can be mutually recursive.
  • When possible, preserve the state of children. If you edit a component, it’s always frustrating if its children unintentionally lose state. As long as the element types of children are defined in other files, we expect their state to be preserved. If they’re in the same file, we should do our best effort.
  • Support custom Hooks. For well-written custom Hooks (some cases like useInterval() can be a bit tricky), hot reloading any arguments (including functions) should work. This shouldn’t need extra work and follows from the design of Hooks. Our solution just shouldn’t get in the way.
  • Support render props. This usually doesn’t pose problems but it’s worth verifying they work and get updated as expected.
  • Support higher-order components. Wrapping export into a higher-order component like connect shouldn’t break hot reloading or state preservation. If you use a component created from a HOC in JSX (such as styled), and that component is a class, it’s expected that it loses state when instantiated in the edited file. But A HOC that returns a function component (potentially using Hooks) shouldn’t shouldn’t lose state even if it’s defined in the same file. In fact, even edits to its arguments (e.g. mapStateToProps) should be reflected.

Feedback

  • Both success and failure should provide visual feedback. You should always be confident whether a hot reload succeeded or failed. In case of a runtime or a syntax error you should see an overlay which should be automatically be dismissed after it is irrelevant. When hot reload is successful, there should be some visual feedback such as flashing updated components or a notification.
  • A syntax error shouldn’t cause a runtime error or a refresh. When you edit the code and you have a syntax error, it should be shown in a modal overlay (ideally, with a click-through to the editor). If you make another syntax error, the existing overlay is updated. Hot reloading is only attempted after you fix your syntax errors. Syntax error shouldn’t make you lose the state.
    A syntax error after reload should still be visible. If you see a modal syntax error overlay and refresh, you should still be seeing it. It categorically should not let you run the last successful version (I’ve seen that in some setups).
  • Consider exposing power user tools. With hot reloading, code itself can be your “terminal”. In addition to the hypothetical // ! command to force remount, there could be e.g. an // inspect command that shows a panel with props values next to the component. Be creative!
  • (partialy)Minimize the noise. DevTools and warning messages shouldn’t expose that we’re doing something special. Avoid breaking displayNames or adding useless wrappers to the debug output.
  • Debugging in major browsers should show the most recent code. While this doesn’t exactly depend on us, we should do our best to ensure the browser debugger shows the most recent version of any file and that breakpoints work as expected.
  • Optimize for fast iteration, not long refactoring. This is JavaScript, not Elm. Any long-running series of edits likely won’t hot reload well due to a bunch of mistakes that need to be fixed one by one. When in doubt, optimize for the use case of tweaking a few components in a tight iteration loop rather than for a big refactor. And be predictable. Keep in mind that if you lose developer’s trust they’ll refresh anyway.
@theKashey
Copy link
Collaborator Author

#1121 would solve p1, and partially 3 others.

@theKashey theKashey added this to the v5 milestone Dec 21, 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

1 participant