aidenybai
feat(react): add `componentDidCatch`
Please describe the changes this PR makes and why it should be merged:
Closes #222 Relates to https://github.com/aidenybai/million-react/issues/6
In draft.
Currently, componentDidCatch in class components only works for render errors, not for effects. It's a lot harder to achieve considering the current architecture of Million, because we need to retrieve recursively the parent VNode of a VNode, until is a class component and has componentDidCatch.
UPDATE
I found a way to catch all errors from effects, but it might add a (lot?) of overhead. In each VNode, we have to keep track of the parent VNode (_parent). Class Components also add a _component field to the root VNode. When an error is thrown from an effect (initial or subsequent), we traverse up the three of VNodes until we find one that has a _component, and run its componentDidCatch. This is similar to Preact's implementation.
Status
- [x] Code changes have been tested against prettier, or there are no code changes
- [x] I know how to update typings and have done so, or typings don't need updating
Semantic versioning classification:
- [x] This PR changes the codebase
- [ ] This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
- [x] This PR changes the internal workings with no modifications to the external API (bug fixes, performance improvements)
- [ ] This PR only includes non-code changes, like changes to documentation, README, etc.
This looks like the best solution for classes. LGTM, merge when you're ready!
I'll research a bit more about catching errors from effects before merging, because componentDidCatch currently only works for (re)render errors.
Pull Request Test Coverage Report for Build 2808056715
- 14 of 54 (25.93%) changed or added relevant lines in 4 files are covered.
- 1 unchanged line in 1 file lost coverage.
- Overall coverage decreased (-0.6%) to 60.402%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| packages/react/hooks.ts | 2 | 14 | 14.29% |
| packages/react/compat.ts | 5 | 33 | 15.15% |
| <!-- | Total: | 14 | 54 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| packages/react/hooks.ts | 1 | 28.29% |
| <!-- | Total: | 1 |
| Totals | |
|---|---|
| Change from base Build 2804120444: | -0.6% |
| Covered Lines: | 1391 |
| Relevant Lines: | 2395 |
💛 - Coveralls
Found an issue with the changes inside the Vite plugin, it was throwing an error and such fail to transform the AST.
Fixed by only adding the _parent to the root velement of a class/function component instead of every velement (previously done in m), and remove the faulty code inside vite-plugin-million.
Left: this PR Right: main branch
