remove is-plain-obj deps

[Jayllyz]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

https://e18e.dev/guide/cleanup.html
https://packages.ecosyste.ms/registries/npmjs.org/packages/is-plain-obj/dependent_packages?order=desc&sort=downloads
Unified is top 2 dependant of is-plain-obj

Solution

Replace by a oneline function :

function isPlainObject(v) {
	return v && typeof v === 'object' && (Object.getPrototypeOf(v) === null || Object.getPrototypeOf(v) === Object.prototype);
}

Alternatives

.

[wooorm]

Hey Anthony!

And then?

Do you use unified?

How far do you want to go? Do you want to inline every dependency?

---

Please at least put some time in your issue and respect our time too. There are of course alternatives and trade offs.

[wooorm]

How well does your code work? I’d assume it might break across realms, due to the Object.prototype?

[Jayllyz]

Hello, thanks for the fast answer.

To be honest, no I don't use unified. I'm just trying to help clean up the js ecosystem even if my contribution is a grain of salt.

"Inline every dependency" isn't what we're trying to do, but some packages shouldn't exist. Take a look at this PR where a contributor saves tons of data by removing is-number as ex.
micromatch/to-regex-range#17

This oneline comes from :

https://github.com/es-tooling/module-replacements/blob/main/manifests/micro-utilities.json

If you don't want to change it that's fine, don't worries.

[remcohaszing]

I personally don’t really think this check is very useful:

unified/lib/index.js

Lines 1254 to 1261 in 35e72b9

	function assertNode(node) {
	// `isPlainObj` unfortunately uses `any` instead of `unknown`.
	// type-coverage:ignore-next-line
	if (!isPlainObj(node) || typeof node.type !== 'string') {
	throw new TypeError('Expected node, got `' + node + '`')
	// Fine.
	}
	}

But I don’t think there’s a way around this check:

unified/lib/index.js

Line 1165 in 35e72b9

if (isPlainObj(currentPrimary) && isPlainObj(primary)) {

I agree removing useless dependencies is a good thing. I don’t necessarily think this is a completely useless dependency. You should look at why a dependency is used and come up with a proposal to replace it. A suggestion to “remove dependency X” isn’t helpful.

[wooorm]

It’s used in case 1 because the dep is already there. The dep is there because of case 2.

[ChristianMurphy]

If there is a runtime performance reason, runtime correctness reason, or security reason to drop the dependency I'm all for it.

The premise of minifying code or documentation to save bandwidth downloading to a developer machine is not one I subscribe to.
Developers should have access to clear easy-to-understand code and documentation.
Code and documentation downloaded from a package manager is cached, it's a one time cost.

For folks who are concerned with the bandwidth, take it up with CI/CD providers and NPM itself to offer more tailored download options, rather than going after individual projects over a few kilobytes.

[Jayllyz]

Thanks for your feedbacks, I see that we have different opinions about such packages (even within e18e)

I don't want to get into a drama/debate because that's not the aim of this initiative. If you want to help or suggest things, I recommend you go to the discord.

Thank you, all the best for your projects and sorry for the time lost.