Comments (11)
https://github.com/researchgate/react-intersection-observer/releases/tag/v1.1.3
from react-intersection-observer.
I experimented with the new version for a bit, but I could not manage to have the error propagated to our error boundary. I'm not sure what the reason is. Previously (1.1.1
) nothing happened, now with 1.1.2
, the component just re-mounts but does not trigger our error behavior.
We display a crash message to the user and hide the faulty part of the page, since it is likely that the error will just happen again on re-mount. That's why we can not just call it a day and have IntersectionObserver
handle the errors.
What is confusing to me is the React docs on error boundaries do not mention re-throwing errors, or how a library component would use an error boundary without affecting unrelated children. I also did a bit of research and could not find anyone else doing something like this. So I think it is a bit unexpected that a library component introduces an error boundary.
I decided to work around the error boundary of the intersection observer by doing:
import {IntersectionObserver} from '@researchgate/react-intersection-observer/lib/es/IntersectionObserver';
I will discuss my workaround with the team and see how it works in production.
If you would find a way to add the IntersectionObserver
without error boundary to your public API, I think that would be great, because this kind of "deeplinking" into packages is usually unexpected. If you tell me how you would call the exported component I could even provide a PR.
from react-intersection-observer.
That is a great way to use this. Thank you!
from react-intersection-observer.
I understand, would you expect the error to be bubbled up if it isn't the one about missing DOM node? That'd be easy to accomplish by rethrowing it for that case.
from react-intersection-observer.
@Rendez Yeah, I think that would be a good solution, thank you for the fast response and the PR! 🎉🎉🎉
On the other hand, I'm not really sure why you brought in the error boundary, do you plan to handle more errors in the future? Otherwise maybe it could even be removed again? 🤔
from react-intersection-observer.
Good question, let me give you a little context... Our query components and async components in general render loading states, e.g:
<Observer onChange={...}>
<Query ...>
{null} // while loading
</Query>
</Observer>
Often this loading state isn't a DOM node but they are null
instead and during development some folks didn't see it (for example, because that component didn't render). This lead to random errors on observe()
calls. The error stack traces and component stack traces of IntersectionObserver.prototype.observe
were very deep, and our devs didn't see initially why these problems appeared, so we could not easily trace back to where errors happened. That's why we decided to start throwing our custom error with a nicer error message.
Up until 1.0.5 we didn't throw but instead of just called the error reporter, and we saw a new error in production (our custom reporter). However, our error was missing the info.componentStack
object that one gets from componentDidCatch
, so again we couldn't trace which of the many instances was causing the issue, and it wasn't that easy to reproduce locally (A/B tests etc).
And this is the story of how we ended up with error boundaries on >=1.1.0.
By the way, the reported errors will still appear in the console, whether you config the errorReporter
or not. If we didn't add our own error boundary, you would need to implement reporting on your error boundaries. That would be fine, if you chose to, we had that before, but again the error message wasn't too clear, and the amount of unmounted nodes was too big.
If you feel this is too protective from the library, then I am interested to know if you'd prefer to have two exports: one guarded observer and one raw one?
from react-intersection-observer.
🎉 This issue has been resolved in version 1.1.2 🎉
The release is available on:
Your semantic-release bot 📦🚀
from react-intersection-observer.
I experimented with the new version for a bit, but I could not manage to have the error propagated to our error boundary. I'm not sure what the reason is. Previously (1.1.1) nothing happened, now with 1.1.2, the component just re-mounts but does not trigger our error behavior.
That is very strange, I added a test case specifically to prove that the parent error boundary would receive the propagated error.
Can you confirm that at least it tries to re-render the component? I didn't try this into production though, but my guess (if the re-render happens for you too) is that the production build doesn't re-throw then, which would be a bit discouraging as that's the point of "re-throwing".
After you give me some more feedback, I think I will cut a release where the default export does no include an error boundary, and perhaps even remove it
from react-intersection-observer.
Okay, super strange: after comparing your test case and then our code and debugging a bit more, I found the reason is apparently that we throw the error in a state update. To test our error boundaries, Sentry integration etc. we have a button in a debugging component. And like in the code I posted above a this.setState
throws an error. When I moved the error throwing into the render()
directly for testing, somehow your error boundary started to re-throw the error.
In both cases it re-renders the component (re-mounts it even I think, because some internal state is reset) so I guess this is really an edge-case and can be ignored? On the other hand, since we did not see any missing node errors before, I think we could still got with the version without the error boundary 🤔
from react-intersection-observer.
That makes sense, error boundaries only catch errors during render. That is within render, cDM, cDU and constructor (I think). The re-mount happens simply because the error boundary we create simply re-renders the passed this.props.children
, your children. These contain now a catchable error.
That said, you're right: I tried your example above (throwing in setState) and what's happening is, the error doesn't get bubbled up because on a re-render there is no following onClick trigger. It has to be a render error for it to be re-thrown automatically. That tells me this is a brittle mechanism and we shouldn't be catching anything in the library. It's unfortunate, but all we can do is to throw our custom error with a better message (back to the original idea).
I'll cut a release without error boundaries as soon as I can. Thanks a lot for your collaboration on this!
from react-intersection-observer.
@Rendez Hey, thanks again for fixing this! Since we have this running in production for some time, I think it works perfectly. We actually have a "enterprise wrapper component" around the library which always inserts a <div>
, so that's why we never run into these exceptions in the first place. Anyways, this library saves us a lot of time so thank you for making it publicly available and also responding to feeback so quickly! ❤️
from react-intersection-observer.
Related Issues (20)
- Warning about missing getDerivedStateFromError in error boundary
- support wrap a class component or functional component HOT 2
- server side
- Migrate to hooks to reduce size HOT 2
- codecov and storybook issues in build HOT 2
- Storybook failing to find variable: exports HOT 1
- Can't observe a React.Suspense element HOT 2
- The story book is broken HOT 1
- Missing type definitions since 1.3.0 HOT 2
- getpooled function is unexpected HOT 3
- npm is invalid on all releases HOT 3
- Not observing when rerendered HOT 6
- Enhancement: Does this actually not work with React v15? HOT 1
- Rm "module": "lib/es/src/index.js", from package.json HOT 5
- Using Yarn to install Library fails HOT 4
- Action Required: Fix Renovate Configuration
- Ability access the ref instance so I can use react-intersection-observer with existing refs HOT 1
- Support React 17; update peerDependencies HOT 7
- Dependency Dashboard
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-intersection-observer.