Skip to content

fix: remove recursion in replaceClose to prevent stack overflow(#106) #107

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Delagen
Copy link

@Delagen Delagen commented Feb 19, 2025

No description provided.

@Delagen
Copy link
Author

Delagen commented Feb 19, 2025

@jorgebucaran Can you approve it and publish? I have some issues with it in my CI, because output seems too large for recurse implementation

@jorgebucaran
Copy link
Owner

Thanks, let me take a look and get back to you. I'm in the middle of a busy week.

@Delagen
Copy link
Author

Delagen commented Feb 24, 2025

@jorgebucaran Any plans on this week? )

@jorgebucaran
Copy link
Owner

Hey, sorry for the delay again. Frankly, I'd be more curious to see exactly how this package is being used to cause a stack overflow first before discarding a well-tested, inmutable, and elegant implementation in favor of an imperative one. I'd just like to see more research go into it.

@Delagen
Copy link
Author

Delagen commented Feb 27, 2025

Hey, sorry for the delay again. Frankly, I'd be more curious to see exactly how this package is being used to cause a stack overflow first before discarding a well-tested, inmutable, and elegant implementation in favor of an imperative one. I'd just like to see more research go into it.

It's used in webpack-dev-server or something linked to output log.

So when log is massive this recursive implementation just produce stack overflow.

I think simple string replace should not use recursion

@jorgebucaran
Copy link
Owner

That's helpful to know. But it might also depend on how the log function is implemented. I'll try to look into it soon. Maybe we can offer an alternative that's still recursive or immutable, without affecting performance.

@Delagen
Copy link
Author

Delagen commented Feb 27, 2025

If program can call stack overflow on small data doesn't matter if code written immutable or not.

Why do you call function with accumulator mutable?

@jorgebucaran
Copy link
Owner

I think I might be able to find a similar solution or alternative. I'll let you know.

@JosephNgabo
Copy link

Am facing this issue when i try to switch from angular 15 to 16

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.

3 participants