feat: make is-bun-module as optional peer dependency

[JounQin]

related #389 (comment)

Technically this is a BREAKING CHANGE, but considering we just raise out v4 recently and this only affects bun users, bun --bun eslint even works without this dependency, so I'd consider this as a minor change.

So for bun users, there are three options:

1. install is-bun-module dependency manually and use bun: true option
2. run eslint with bun --bun eslint w/o bun: true option
3. enable run#bun in bunfig.toml w/o bun: true option

cc @SukkaW @SunsetTechuila

[changeset-bot]

🦋 Changeset detected

Latest commit: 4ddad73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
eslint-import-resolver-typescript	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[socket-security]

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/buffer@5.7.1, npm/callsites@3.1.0, npm/defaults@1.0.4, npm/is-bun-module@1.3.0, npm/path-key@3.1.1, npm/restore-cursor@3.1.0, npm/shebang-command@2.0.0, npm/string-width-cjs@4.2.3, npm/strip-ansi-cjs@6.0.1, npm/wrap-ansi-cjs@7.0.0, npm/wrap-ansi@8.1.0

View full report↗︎

[github-actions]

size-limit report 📦

Path	Size
lib/index.js	1.4 KB (-6.32% 🔽)

[SunsetTechuila]

my opinion is that this package should ignore bun modules by default, this is something i personally would expect

anyway

pros of making is-bun-module optional:

• saves 25kb of disc space and 7.7kb of network data (if the user doesn't have semver in (sub)dependencies, which is very unlikely)

cons:

• leads to a more complicated code
• potentially creates hassle for the users

i'm also considering reworking my package, so you would be able to just do

if (module.isBuiltin(source) || isBunBuiltin(source)) {
  log('matched core:', source)
  return { found: true, path: null }
}

instead of all of this:

eslint-import-resolver-typescript/src/index.ts

Lines 65 to 82 in 458fdaf

	let bunVersion = process.versions.bun as Version | undefined
	// don't worry about bun core modules
	if (bunVersion || options.bun) {
	if (
	bunVersion
	? module.isBuiltin(source)
	: isBunModule(source, (bunVersion = 'latest')) ||
	isSupportedNodeModule(source, bunVersion)
	) {
	log('matched bun core:', source)
	return { found: true, path: null }
	}
	} else if (module.isBuiltin(source)) {
	// don't worry about node core modules
	log('matched node core:', source)
	return { found: true, path: null }
	}

[JounQin]

my opinion is that this package should ignore bun modules by default, this is something i personally would expect

Doesn't it?

if the user doesn't have semver in (sub)dependencies, which is very unlikely

Yeah, and alo multiple incompatible versions can be installed unfortunately.

i'm also considering reworking my package, so you would be able to just do

Great! I'm holding on in favor of this then.

[JounQin]

That's why bun?: boolean option is added? Non-bun users should not try to resolve bun modules otherwise there could be unexpected behavior.

[JounQin]

I refactor the codes a little, no process.versions.bun check needed anymore.

The refactored codes are much cleaner now, no need to waiting is-bun-module changes.

[SunsetTechuila]

isBunBuiltin(source)

@JounQin need your opinion: should this function return true for node built-in modules that are not implemented in bun? and so, should eslint-import-resolver-typescript resolve such modules?

[SunsetTechuila]

module.isBuiltin returns true for non-implemented node modules and mdoule.builtinModules contains them

[JounQin]

module.isBuiltin returns true for non-implemented node modules and mdoule.builtinModules contains them

@SunsetTechuila I personally think it should return false, and that's why I split bun condition with node core previously. And that would help bun users avoid using non-implemented features.

But with that change, is-bun-module seems can not be optional for bun anymore? Because module.isBuiltin is incorrect in this case.

[SunsetTechuila]

is-bun-module seems can not be optional

i think so. i would expect eslint-import-resolver-typescript to detect if i'm running it in bun and use is-bun-module for checking if given module is a built-in

[SunsetTechuila]

i'm also considering reworking my package

this is what i have right now: SunsetTechuila/is-bun-module@66d9385

[JounQin]

i think so. i would expect eslint-import-resolver-typescript to detect if i'm running it in bun and use is-bun-module for checking if given module is a built-in

Sure it should! Let's add it back then, would you like to raise a PR for this?

[SunsetTechuila]

would you like to raise a PR for this?

i will do so as soon as my package is ready