Ethanfann/remove trim off newlines dependency #2825
Conversation
lib/trim-off-newlines.js
Outdated
| @@ -0,0 +1,4 @@ | |||
| export default function trimOffNewlines(text) { | |||
| const regex = '/^[\r\n]+|[\r\n]+$/g'; | |||
There was a problem hiding this comment.
Make it a regex literal and just put it inline in the replace call.
There was a problem hiding this comment.
I've updated the PR with your suggestion 👍
test/helpers/trim-off-newlines.js
Outdated
| import test from '@ava/test'; | ||
|
|
||
| import trimOffNewlines from '../../lib/trim-off-newlines.js'; |
There was a problem hiding this comment.
| import test from '@ava/test'; | |
| import trimOffNewlines from '../../lib/trim-off-newlines.js'; | |
| import test from '@ava/test'; | |
| import trimOffNewlines from '../../lib/trim-off-newlines.js'; |
There was a problem hiding this comment.
I've update the PR with your suggestion 👍
|
Tests seem weird. As if it expected to remove all newlines while the regex is only to trim them from both ends of a string. (I'm looking from mobile atm so I might be wrong, sorry if that's the case.) Upd: > '\na\nb \n c\n'.replace(/^[\r\n]+|[\r\n]+$/g, '')
< 'a\nb \n c'
> '\na\nb \n c\n'.replace(/[\r\n]+/g, '')
< 'ab c' |
|
Thanks @KillyMXI! You're right. Not only was the test faulty, the test file wasn't even being picked up when the suite ran. Apologies for this and I've updated the PR with the fixed tests. |
|
I'd still keep a test for inner newlines being unaffected though. |
|
Added tests with inner newlines 👍 |
|
@ethanfann thanks for the PR! I had a look at where / how we use this dependency and I reckon we can remove it entirely. See #2826. |
In response to https://nvd.nist.gov/vuln/detail/CVE-2021-23425, this PR removes the
https://www.npmjs.com/package/trim-off-newlinespackage in favor of a utility function.Tests have been added to
test/helpers/trim-off-newlines.jstest/trim-off-newlines/test.jswith various cases related to Windows, Mac, and Linux operating systems.Some test failures are occurring with
test-tap/integration/watcher.js, however, I do not know if they are related to the removal of the previous trim packages library in favor of the current. Please let me know if there is a case that the new function isn't handling.