dlmanning / gulp-sass

SASS plugin for gulp
MIT License
1.56k stars 381 forks source link

Replace lodash with the lodash.clonedeep package #818

Closed XhmikosR closed 2 years ago

XhmikosR commented 2 years ago

Notes:

  1. lodash isn't in any of the current dependencies' deps
  2. I'm unsure if the individual packages are still maintained, though. Alternatively, we could find another package.

https://packagephobia.com/result?p=lodash%2Clodash.clonedeep

XhmikosR commented 2 years ago

I pushed another patch which switches to the extend package. I don't have a preference, both should do the job and are small.

https://packagephobia.com/result?p=lodash%2Clodash.clonedeep%2Cextend

XhmikosR commented 2 years ago

The CI failure is unrelated to these changes; GH Actions return the paths as D or d for some reason inconsistently sometimes.

XhmikosR commented 2 years ago

@xzyfer can you let me know how I should proceed please?

XhmikosR commented 2 years ago

But with which solution 😛?

xzyfer commented 2 years ago

I'm happy with either. It's fairly inconsequential as long as tests pass.

XhmikosR commented 2 years ago

The thing is that tests don't catch everything :)

Anyway, I went with extend because it's also a dependency of other packages one will probably have:

C:\Users\xmr\Desktop\gulp-sass>npm ls extend
gulp-sass@5.0.0 C:\Users\xmr\Desktop\gulp-sass
+-- extend@3.0.2
+-- gulp@4.0.2
| +-- gulp-cli@2.3.0
| | `-- liftoff@3.1.0
| |   `-- extend@3.0.2  deduped
| `-- vinyl-fs@3.0.3
|   `-- glob-stream@6.1.0
|     `-- extend@3.0.2  deduped
`-- node-sass@6.0.1
  `-- request@2.88.2
    `-- extend@3.0.2  deduped

lodash.clonedeep:

C:\Users\xmr\Desktop\gulp-sass>npm ls lodash.clonedeep
gulp-sass@5.0.0 C:\Users\xmr\Desktop\gulp-sass
`-- eslint@7.32.0
  `-- table@6.7.2
    `-- lodash.clonedeep@4.5.0
xzyfer commented 2 years ago

Good enough reason as any :)

On Wed, 6 Oct 2021, 5:42 pm XhmikosR, @.***> wrote:

The thing is that tests don't catch everything :)

Anyway, I went with extend because it's also a dependency of other packages one will probably have:

C:\Users\xmr\Desktop\gulp-sass>npm ls extend @. C:\Users\xmr\Desktop\gulp-sass +-- @. +-- @. | +-- @. | | -- ***@***.*** | |-- @. deduped | `-- @. | -- ***@***.*** |-- @. deduped `-- @. -- ***@***.*** -- @.*** deduped

lodash.clonedeep:

C:\Users\xmr\Desktop\gulp-sass>npm ls lodash.clonedeep @. C:\Users\xmr\Desktop\gulp-sass `-- @. -- ***@***.*** -- @.***

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dlmanning/gulp-sass/pull/818#issuecomment-935585712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAENSWGNWXOXH6WNLOPYYB3UFPVWJANCNFSM5FIS4PCQ .

XhmikosR commented 2 years ago

NVM, I think we better play it safe and go with lodash.clonedeep. We can revisit this later.