Skip to content

unwrap obj to clone it#6409

Merged
6543 merged 8 commits into
woodpecker-ci:mainfrom
6543-forks:fix/lodash
Apr 15, 2026
Merged

unwrap obj to clone it#6409
6543 merged 8 commits into
woodpecker-ci:mainfrom
6543-forks:fix/lodash

Conversation

@6543

@6543 6543 commented Apr 8, 2026

Copy link
Copy Markdown
Member

@6543 6543 requested a review from qwerty287 April 8, 2026 16:56
@6543 6543 added the regression fix a bug that was not released yet label Apr 8, 2026
@6543 6543 mentioned this pull request Apr 8, 2026
@6543 6543 added the wip label Apr 8, 2026
@6543

6543 commented Apr 8, 2026

Copy link
Copy Markdown
Member Author

still get errors :/

xoxys
xoxys previously approved these changes Apr 9, 2026
@xoxys xoxys dismissed their stale review April 9, 2026 07:23

Not working

@6543

6543 commented Apr 9, 2026

Copy link
Copy Markdown
Member Author

jep... :/

@6543 6543 removed the wip label Apr 9, 2026
Comment thread web/src/lib/utils/index.ts
@6543 6543 added the ui frontend related label Apr 9, 2026
@6543 6543 requested a review from xoxys April 9, 2026 19:16
@qwerty287

Copy link
Copy Markdown
Contributor

For the showAddRegistry funcs you can actually use structedClone, that works. It's only about the edit function

@6543

6543 commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

well i dont understant why it works or dont in certain cases, i would just stick to our clone func for data for redability

@6543 6543 mentioned this pull request Apr 14, 2026
16 tasks
@xoxys

xoxys commented Apr 15, 2026

Copy link
Copy Markdown
Member

Why not just copy the cloneDeep func from lodash for now?

@6543

6543 commented Apr 15, 2026

Copy link
Copy Markdown
Member Author

it does to much, we just want to copy json object, it can copy objects with functions in it etc...

... this func of lowdash is huge and we dont ned it

https://github.com/lodash/lodash/blob/cb0b9b9212521c08e3eafe7c8cb0af1b42b6649e/lodash.js#L2667

@xoxys xoxys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested. If it works fine for me. However Ill count this as another micro optimization that caused more trouble then it helped. Personally I dont care about a bigger binary size and even if lodash has sec issues (go stdlib had a lot of critical cves as well in the past months) we are mostly not affected as we dont use the vulnarable code.

@6543 6543 merged commit ed4a7de into woodpecker-ci:main Apr 15, 2026
6 of 7 checks passed
@6543 6543 deleted the fix/lodash branch April 15, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

regression fix a bug that was not released yet ui frontend related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants