search

eslint / json

JSON language plugin for ESLint
Apache License 2.0
18 stars 2 forks source link

fix: Don't require ESLint #25

Closed nzakas closed 3 weeks ago

nzakas commented 1 month ago

Prerequisites checklist

What is the purpose of this pull request?

Make ESLint an optional peer dependency.

What changes did you make? (Give an overview)

Added peerDependenciesMeta to package.json in order to set eslint as an optional peer dependency.

Practically speaking, the plugin doesn't require ESLint to be useful. We are using it in the Code Explorer without ESLint, where it's installation is causing npm errors because Code Explorer uses ESLint v8.x.

Related Issues

Is there anything you'd like reviewers to focus on?

It seems like we shouldn't block installation of this package even if ESLint isn't installed or ESLint v8.x is installed, but let me know if anyone has a different opinion. From what I can tell, this change won't have any noticeable effect to most users of this plugin.

fasttime commented 1 month ago

It seems like we shouldn't block installation of this package even if ESLint isn't installed or ESLint v8.x is installed, but let me know if anyone has a different opinion.

I think that to allow installing ESLint v8.x without the --force flag, the peerDependencies version for eslint should be changed to something like "^8.0.0 || ^9.6.0". Currently it's "^9.6.0".

https://github.com/eslint/json/blob/08fd5d3c41f015689561af762dbc6eec66592add/package.json#L71

Making the peer dependency optional will allow a package to install @eslint/json without eslint, but not with a version of eslint which is out of range.

For the same reason in eslint we have the jiti optional peer dependecy version set to "*" and not "^1.21.0", so consumers that use jiti for other purposes than to support eslint are free to install any version they want.

From what I can tell, this change won't have any noticeable effect to most users of this plugin.

Probably most users of this plugin will already have a new versioneslint declared in their devDependencies. So yes, there shouldn't be any noticeable effects. I'm fine with merging this as a fix: 👍

nzakas commented 1 month ago

Making the peer dependency optional will allow a package to install @eslint/json without eslint, but not with a version of eslint which is out of range.

Hmm, not what we want. I wonder if it's best just remove the peer dependency altogether?

fasttime commented 1 month ago

Hmm, not what we want. I wonder if it's best just remove the peer dependency altogether?

Makes sense I think, if it should be possible to install this package irrespective of eslint.