Skip to content

fix: properly handle render errors in 'react' driver.#335

Open
tmilar wants to merge 4 commits into
krakenjs:mainfrom
tmilar:fix/react-driver
Open

fix: properly handle render errors in 'react' driver.#335
tmilar wants to merge 4 commits into
krakenjs:mainfrom
tmilar:fix/react-driver

Conversation

@tmilar

@tmilar tmilar commented Feb 2, 2021

Copy link
Copy Markdown

With this change, a react component that throws on the render() async method, will check first if the component is still mounted, and in that case attempt to rethrow the error inside the React render loop.

Fixes #334

@bluepnume

Copy link
Copy Markdown
Contributor

This looks great to me, thanks for taking this up! Can you add a couple of tests to verify the new behavior?

@tmilar

tmilar commented Feb 3, 2021

Copy link
Copy Markdown
Author

This looks great to me, thanks for taking this up! Can you add a couple of tests to verify the new behavior?

Thanks! I'll try to work on that when I get some free time.
Since I'm on Windows I've been having a really though time making the project work with all the npm scripts intended for UNIX environments :(

@tmilar
tmilar force-pushed the fix/react-driver branch 2 times, most recently from f038f7e to 645faac Compare February 4, 2021 17:40
@bluepnume

Copy link
Copy Markdown
Contributor

Totally, sorry about the poor Windows support. If there are any changes you would like to include to improve cross-compatibility there, I'd be happy to merge them in.

@tmilar
tmilar force-pushed the fix/react-driver branch 2 times, most recently from 06ef90e to cf61b10 Compare February 5, 2021 00:01
@danwetherald

Copy link
Copy Markdown

When will this be released? Seems like a major issue that is not being taken seriously with the priority it should have.

@bluepnume
bluepnume force-pushed the master branch 5 times, most recently from a6dbeec to f8eeb3e Compare September 10, 2021 07:52
@bluepnume
bluepnume force-pushed the master branch 2 times, most recently from 22cbfdf to c890107 Compare September 22, 2021 06:36
@badeAdebayo

Copy link
Copy Markdown

will this ever be merged? @tmilar @bluepnume

@yongzhihuang

Copy link
Copy Markdown

Ping @tmilar @bluepnume ^^ 🙏

@Karamavrov

Copy link
Copy Markdown

@tmilar @bluepnume

Hey hey. Thank you for your work!
But the issue is still occurring, it's 3-years past the PR creation. Can we get some sort of love and make merge happen? 🙏

@leeran7

leeran7 commented Apr 8, 2025

Copy link
Copy Markdown

Anything on this?

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

Successfully merging this pull request may close these issues.

Removing the container element after CLOSE/DESTROY zoid event (after close()), results in uncaught error

7 participants