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

Errors in the useReduce reducer function are thrown differently than React #2750

Closed
mischnic opened this issue Sep 11, 2020 · 2 comments
Closed

Comments

@mischnic
Copy link

mischnic commented Sep 11, 2020

Reproduction

https://codesandbox.io/s/preact-usereducer-error-lr1mx?file=/src/index.js

import React, { useEffect } from "preact/compat";
import ReactDOM from "preact/compat";
/*/
import React, { useEffect } from "react";
import ReactDOM from "react-dom";
*/

function useTest() {
  try {
    let [, dispatch] = React.useReducer(() => {
      throw new Error("X");
    }, "a");

    return {
      load() {
        dispatch("b");
      }
    };
  } catch (e) {
    console.log("useReducer", e.message);
    throw e;
  }
}

function Test() {
  let { load } = useTest();
  useEffect(() => {
    try {
      load();
    } catch (e) {
      console.log("dispatch", e.message);
      throw e;
    }
  });

  return "Test";
}

class Catcher extends React.Component {
  constructor() {
    super();
    this.state = { error: null };
  }

  componentDidCatch(error) {
    this.setState({ error: error.message });
  }

  render() {
    if (this.state.error) {
      return <p>Something went badly wrong: {this.state.error}</p>;
    }
    return this.props.children;
  }
}

function App() {
  return (
    <Catcher>
      <Test />
    </Catcher>
  );
}

ReactDOM.render(<App />, document.getElementById("root"));

Steps to reproduce

Open reproduction

Expected Behavior

🤷‍♂️ (not sure how far you want to take the "1:1" compatibility to React 😄 )

Actual Behavior

An error is thrown in the reducer function.

Preact throws this error back through the the dispatch() call (so load() in this case). ("dispatch" is logged).

React throws this error back "through the the useReduce() call". ("useReducer" is logged).

Context

This messes with @testing-library/preact-hooks-testing-library because you need to catch the actual calls and can't rely on result.error. testing-library/preact-hooks-testing-library#3

@kitten
Copy link
Contributor

kitten commented Sep 25, 2020

Not sure if you want to address this, but it may be worth doing so; it's a subtle difference but dispatching in useState and useReducer adds to a queue that is only processed on the next render call: https://github.com/facebook/react/blob/480626a9e920d5e04194c793a828318102ea4ff4/packages/react-dom/src/server/ReactPartialRendererHooks.js#L279-L310

Also posting the range in question from react-ssr-prepass here since they're cleaned up of debugging calls and hence quicker to read: https://github.com/FormidableLabs/react-ssr-prepass/blob/e2020e1e4ab2a34aa2141403902fe7f17343f32b/src/internals/dispatcher.js#L192-L211
And dispatchAction putting the linked list update on top of a separate queue: https://github.com/FormidableLabs/react-ssr-prepass/blob/e2020e1e4ab2a34aa2141403902fe7f17343f32b/src/internals/dispatcher.js#L269-L274

This could be a very minor addition to hooks/index.js that wouldn't make a huge difference but improve consistency as well.

Edit: Just editing to state that I'm wrong; this could be more tricky, since state changes like these would also still have to be aborted if the state remains the same. React does this by marking the current fiber as potentially unchanged. In Preact this may not be possible as the render phase has already begun and can't be interrupted as it's not split into two phases as far as I'm aware.

@JoviDeCroock
Copy link
Member

Closing this as a dupe of #2913 (comment)

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