Skip to content

Drop extend CJS dependency - #273

Closed
fuma-nama wants to merge 3 commits into
unifiedjs:mainfrom
fuma-nama:main
Closed

fuma-nama wants to merge 3 commits into
unifiedjs:mainfrom
fuma-nama:main

Conversation

@fuma-nama

@fuma-nama fuma-nama commented May 20, 2026

Copy link
Copy Markdown

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

Removed the remaining CJS dependency extend, since unified itself is ESM-only, it would be great if all deps are also ESM so it doesn't need bundling to work in browser environments.

The extend package is being used as a deep merge utility, while its core functionality is being replaced by Object.assign(). I think a proper deep merge utility would do better, but extend has some behaviours that may differs from existing deep merge utilities, so I rewrote the original utility with modern ESM, shouldn't cause any breaking changes.

I had to lower the test coverage as some edge cases are not easily reached, let me know if this isn't desirable.

@github-actions github-actions Bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels May 20, 2026

@Murderlon Murderlon 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.

Not up to me to decide whether this goes in. Knowing @wooorm, he really likes consistency. So that usually means using extend everywhere or nowhere, not just dropping in one package. Luckily it seems there are only two

Comment thread package.json Outdated
Comment thread lib/deep-merge.js
@fuma-nama

fuma-nama commented May 20, 2026

Copy link
Copy Markdown
Author

Oops I didn't search in other repositories, I think that will need another PR, would like to hear some inputs from maintainers before doing that. extend doesn't actually merge data structures like Map & Set, there's better alternatives for deep merge utils.

@Murderlon

Copy link
Copy Markdown
Member

Taking a step back, wondering if we can use structuredClone, browser support is good, also in Node.js since 17.0.0 🤔

@fuma-nama

Copy link
Copy Markdown
Author

I think some libraries might store functions in the settings object, I would prefer to not break anything when possible.

@wooorm

wooorm commented Jun 3, 2026

Copy link
Copy Markdown
Member

Sorry but I don‘t think this is possible.

I tried structuredClone. It passed tests. It worked in several of the 100s of repos. Then in failed in some. Some things put functions on data.
There are lots of people writing their own plugins and doing all kinds of things.

This requires lots and lots of tests to have parity with extend.
At that point, put it in a new repo, share it with the world. Maybe then it can be used here.

See GH-263, GH-153

@wooorm wooorm closed this Jun 3, 2026
@github-actions

This comment has been minimized.

@wooorm wooorm added the 👀 no/external This makes more sense somewhere else label Jun 3, 2026
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Hi team! Could you describe why this has been marked as external?

Thanks,
— bb

@github-actions github-actions Bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on

Development

Successfully merging this pull request may close these issues.

3 participants