-
Notifications
You must be signed in to change notification settings - Fork 482
fix(lib): correct the concat method in TokenizedStringFragments #3772
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
Conversation
Hey, thanks for your help! Could you add a test case that shows which user-facing issue this fix solved so that we don't reintroduce the problem by accident in the future? |
Hi, thank you for your feedback! I have written a test case for the concat method to verify the user-facing issue is resolved. Could you confirm where exactly I should add the test code? I assume it should go into |
7cdf1be
to
453cb0c
Compare
453cb0c
to
17e0ce2
Compare
17e0ce2
to
f0c8272
Compare
Hi there! 👋 We haven't heard from you in 60 days and would like to know if you're still working on this or need help. If we don't hear from you before then, I'll auto-close this PR in 30 days. |
The concat method was not properly updating `this.fragments` because `Array.prototype.concat` does not modify the original array in place. Changed the implementation to use `push` with the spread operator to correctly merge fragments.
Added a test case to verify that the `concat` method correctly merges fragments from another instance.
bb927b6
to
eb67df8
Compare
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
The
concat
method was not properly updatingthis.fragments
becauseArray.prototype.concat
returns a new array and does not modify the original array in place. Changed the implementation to usethis.fragments.push(...other.fragments);
to correctly merge the fragments intothis.fragments
.Description
I thought
concat
method inTokenizedStringFragments
was intended to merge fragments from another instance intothis.fragments
. However, it was not functioning correctly becauseArray.prototype.concat
returns a new array and does not modify the original array in place.To fix this issue, I updated the
concat
method to usethis.fragments.push(...other.fragments);
. This modifiesthis.fragments
in place and ensures that all fragments are correctly merged.Checklist