Totally remove lodash #1247
Totally remove lodash #1247
Conversation
Replace lodash functions by ESNext style functions or by deepmerge-ts (lighter and more maintened)
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
We use npm, please remove this file and run npm install so package-lock is in sync.
`deepmerge-ts` seems to have an [issue](RebeccaStevens/deepmerge-ts#488) with a type file. Skipping lib check in `tsconfig.json` ([doc](https://www.typescriptlang.org/tsconfig/#skipLibCheck)) works around that and will increase compile time without drawbacks.
|
Seems like tests do not quite work. |
| @@ -368,7 +367,7 @@ export class Client extends EventEmitter { | |||
| if (!output || !output.$lookupTypes) { | |||
| debug('Response element is not present. Unable to convert response xml to json.'); | |||
| // If the response is JSON then return it as-is. | |||
| const json = _.isObject(body) ? body : tryJSONparse(body); | |||
| const json = body !== null && typeof body === 'object' ? body : tryJSONparse(body); | |||
There was a problem hiding this comment.
body can be undefined, should be something like
const json = body && typeof body === 'object' ? body : tryJSONparse(body);
|
I was thinking about this change and don't see any benefits in removing lodash. While it was not updated for 4 years, seems like version 5 will be released at some point. After my recent changes node-soap does not contain any vulnerabilities. If you have different point of view ii I happy to discuss it. |
|
I will update this PR after summer break. Concerning the why, I have 2 reasons to remove this kind of dependency:
I understand those are very opinionated reasons and I'll understand if you do not wish to remove it. |
|
Hi @JbIPS, Okay, happy to discuss proposed solution. |
|
I had another look on it ... that is not that easy, deepmerge-ts is not enough. Some stuff can be replaced with native JS features, but not all. Too much work to replace what works just fine. |
Replace lodash functions by ESNext style functions or by deepmerge-ts (lighter and more maintened)