-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: safer string truncation #1478
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@daibhin since this would maybe help with your upgrade PR... I'm considering adding a (lazy loaded?) polyfill for array.from |
Size Change: +1.66 kB (+0.05%) Total Size: 3.18 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly I'm still seeing failures on IE11 in #1276 despite my changes :/ Regardless, this seems good
NO, that polyfill definitely does not work 🤣 |
interesting... looks like the array.from polyfill in IE11 bundle makes this timeout 🤯 |
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
We truncate strings by slicing them at a given length both in replay and in general capture property processing
String.prototype.slice()
andString.prototype.substring()
are not unicode awareso
Major browsers and Node 20 have
toWellFormed
Our CI/tests/etc don't use Node 20 so even though we could use toWellFormed in prod it would be a pain to add. Moving to Node20 would probably fix some people's builds and break others. Let's avoid confusion for now.
Luckily the one consistent thing about JS is that it is not consistent.
Array.from(someString)
is unicode aware and correctly splits a string into an array of valid characters. So we can then slice that array. To get a string that is sort-of the right length.Our string truncation in the JS is set arbitrarily so it doesn't matter that this mechanism returns strings that may not have a
.length
that exactly matches themaxLength
passed to truncation