Stack Overflow Exception when calling ToObject on ObjectInstance containing a reference to itself about jint HOT 9 CLOSED

Thank you so much! I will check those out and try my best! :D

from jint.

Well this was an easy one! 😉

from jint.

A PR to fix with a test case would be much appreciated.

from jint.

@lahma I would love to give it a go at trying to fix it but it would be my first contribution (to any open source), is there a way to assign myself (or remove assignment if I can't fix it) for this issue so I don't spend time looking into it and someone else fixes it haha? 😂 Any rules or what not? Sorry for noob questions ^^

from jint.

Hey there's always the first time! Let's try to make it a positive experience.

There's some general guidance for pull requests https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request , a takeaway could be creating a separate branch and work from there.

Start with a failing test case and then debug what's happening. Try to follow coding conventions in code base even if they are not something that you usually follow.

Create a PR to showcase progress and don't hesitate to ask for information!

from jint.

@lahma The area of code I would need to alter to fix the issue would probably benefit from a bit of refactoring first (at least according to me) :3 Is it okay and/or encouraged to make PRs with just refactorings (not rewrites though)? In this case just splitting up a long method into smaller methods or maybe even some static classes? :3

from jint.

Pretty hard to say as I don't know what is the starting point and the goal. Maybe we can discuss the outcome via the PR.

from jint.

That sounds like a plan! Basically it's a long switch case for a lot of different types to convert and it would probably be nice to have a method for each case or something rather than each converter code inside the case block ^^ I am fairly sure the bug itself lies within the object/array case block. But I'll see if I can put a refactor PR together and we can discuss it there! if we don't like it we can just close it!

from jint.

After finally getting home to my computer and actually started writing some code I believe we can close this issue. This bug has probably been fixed after the version I tested it on because the exact same code which made my application throw a StackOverflow exception instead threw a JavascriptException (Cyclic reference detected.).

from jint.