refactor: replace deprecated String.substr with String.slice - #7662
Conversation
Sorry, something went wrong.
Fixes plotly#6078 String.prototype.substr is deprecated and should be replaced with String.prototype.slice or String.prototype.substring. This commit replaces all occurrences of .substr() with .slice() across the codebase. The slice method is preferred as it handles negative indices more intuitively and matches the existing usage patterns.
- Fixes test failure in svg_text_utils_test.js where slice(0, negative) returned a string instead of empty string (unlike substr). - Fixes similar potential issues in rangeslider/defaults.js and cartesian/axes.js using Math.max(0, ...).
camdecoster
left a comment
There was a problem hiding this comment.
Thanks for the PR! It looks like there a handful of build files that also use substr. Could you update those as well? You can ignore anything in stackgl_modules. Other than that, this looks good.
Sorry, something went wrong.
Fixed remaining deprecated substr() calls in build/test files: - test/strict-d3.js - test/jasmine/assets/domain_ref_components.js - test/jasmine/bundle_tests/no_webgl_test.js - test/jasmine/assets/custom_assertions.js - test/jasmine/bundle_tests/plotschema_test.js Excluded stackgl_modules as requested.
|
Done! Found and fixed all the remaining substr calls in the build files (excluding stackgl_modules like you mentioned). Updated 5 files total. |
Sorry, something went wrong.
|
Hey, while fixing those I noticed there are a few more substr calls in test/jasmine/tests/ (like snapshot_test.js, toimage_test.js, axes_test.js, etc.). Want me to update those too or are those fine to leave as-is? |
Sorry, something went wrong.
|
Ah, I had those filtered from my search. Yeah, go ahead. Might as well clean it all up. |
Sorry, something went wrong.
|
Just curious: why did you go with substring instead of slice for the test files? |
Sorry, something went wrong.
|
swapped substr to substring only because of a quick search/replace, not because of a deliberate choice. There’s no functional reason for substring here — shall I update these to slice as well to keep everything consistent ? |
Sorry, something went wrong.
|
Go ahead and use slice. That way we won't have to deal with the slight differences between the two methods. |
Sorry, something went wrong.
Replaced substring() with slice() in test files to maintain consistency with the source code changes where substr() was replaced with slice().
|
One last thing: could you please add a draftlog? |
Sorry, something went wrong.
|
I added a draftlog, Is everything good now? |
Sorry, something went wrong.
|
Thanks for the contribution! There are plenty of other opportunities to update the syntax or switch from deprecated methods. We'd love to have more help getting things modernized. |
Sorry, something went wrong.
Fixes #6078
Summary
This PR replaces all occurrences of the deprecated String.prototype.substr() method with String.prototype.slice() across the codebase.
Changes
Why slice over substring?
Testing