Skip to content
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

fix: prevent extendBaseUrl recursion #2621

Merged

Conversation

BobbieGoede
Copy link
Collaborator

@BobbieGoede BobbieGoede commented Dec 14, 2023

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Fixes the recursive wrapping of nuxtI18nOptions and investigated and described here #2034 (comment).

This PR #2620 by @s00d would have fixed this issue as well (credited in commit), I opted for making a copy of nuxtI18nOptions instead, to ensure the options are not mutated elsewhere by requests.

I will publish a new release after this is merged, after which I think we can close the issues related to the memory leak.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Co-authored-by: Pavel Kuzmin <s00d@users.noreply.github.com>
@BobbieGoede BobbieGoede self-assigned this Dec 14, 2023
@s00d
Copy link
Contributor

s00d commented Dec 14, 2023

I have checked it on my side, and now everything is working fine. The issue is resolved, and your approach is even better than my solution. This will help avoid problems in other areas as well.

@s00d s00d mentioned this pull request Dec 14, 2023
7 tasks
@BobbieGoede BobbieGoede requested review from kazupon and removed request for kazupon December 14, 2023 11:01
Copy link
Collaborator

@dargmuesli dargmuesli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking very good with my project. The main leak is gone now, only some left but I guess those are on me πŸ˜‰

@BobbieGoede BobbieGoede merged commit f4fed2f into nuxt-modules:main Dec 14, 2023
7 checks passed
@BobbieGoede
Copy link
Collaborator Author

Looking very good with my project. The main leak is gone now, only some left but I guess those are on me πŸ˜‰

That's good to hear, I'll publish a release in a bit, it worries me that the latest release contains this recursion issue πŸ˜₯.. I still do think there are leaks, but I haven't found a reliable way to reproduce them.

DarthGigi pushed a commit to DarthGigi/i18n that referenced this pull request Apr 16, 2024
Co-authored-by: Pavel Kuzmin <s00d@users.noreply.github.com>
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.

None yet

3 participants