ljharb / jsonify

4 stars 2 forks source link

Please explain the use case for this package in the readme #3

Open everett1992 opened 1 year ago

everett1992 commented 1 year ago

This is a 0.0.1 library with 6 million weekly downloads that provides the same functionality as the ECMAScript built in JSON, I would love to see a 'why this is necessary' explanation in the README.

everett1992 commented 1 year ago

json-stable-stringify is a common consumer of jsonify in my packages and this line suggests fallback to jsonify when JSON is not defined - are there JavaScript runtimes that don't define JSON?

Would it be possible to recommend jsonify as an optional dependency?

ljharb commented 1 year ago

There certainly are older engines that don't - IE 6 and 7, for example - but also, delete JSON is a thing.

It's not possible for ANY package author to accurately recommend the "category" of dependency - every single package is sometimes a peer dep, sometimes a runtime dep, sometimes a dev dep, sometimes an optional dep, etc.

Also, "optionalDependencies" are only for things that might fail to install, so specifying this package as an optional dep wouldn't serve any purpose.

everett1992 commented 1 year ago

optional would help in my case because the private npm registry I use has blocked jsonify based on it's license.

I wish npm has something like cargo's features to enable or disable optional deps. An option could be to not declare any dependency on jsonify and print a useful message when it is required.

let stringify;
if (typeof JSON !== 'undefined') {
  stringify = JSON.stringify
} else {
  try {
    stringify = require('jsonify').stringify
  } catch (ex) {
    throw Error(`runtime does not provide JSON, install jsonify for stringify support`)
  }
}
var jsonStringify = (typeof JSON !== 'undefined' ? JSON : require('jsonify')).stringify;
ljharb commented 1 year ago

Can you elaborate on the issues with the license?

Either way, you can use npm overrides to replace jsonify with a custom package that only exports JSON.parse, if you like, and then jsonify wouldn't even get requested.

everett1992 commented 1 year ago

Our registry does not like Public Domain as a declared license. Probably because it's not a spdx license id? That's an issue on my end, but a lot of packages were failing to install because jsonify was missing which caused me to look into it and it caused left-pad flashbacks seeing 0.0.1 and 6M weekly downloads on a package I could not understand the need for.

ljharb commented 1 year ago

I think that if your registry is blocking on undesirable licenses - versus having a CI check that fails on undesirable licenses - you're signing yourself up for a ton of problems.

Your internal registry should include everything, regardless.

albertodiazdorado commented 7 months ago

Hi @everett1992 , if jsonify arrived at your project via swagger-ui, just add a module resolution. Module resolutions are yarn-specific, but I am sure that npm has something similar:

  "resolutions": {
    "patch-package": "^7"
  }
albertodiazdorado commented 7 months ago

@ljharb The German law (and all legal systems derived from it, which are most of European ones) does not recognize anything such as "Public Domain". Using your package in an European country other than Great Britain is a pain in the a** if you want to be compliant with OSS guidelines.

I think this is why most people working in Europe have a lot of issues working with jsonify

albertodiazdorado commented 7 months ago

@everett1992 if jsonify arrived to your project a dependency other than patch-package (probably via swagger-ui), you can just override it with some local package.

For example:

mkdir german-law-does-not-recognise-public-domain
pushd german-law-does-not-recognise-public-domain
yarn init -y
echo "module.exports = { stringify: JSON.stringify, parse: JSON.parse }" >> index.js
popd

And then add this to your package.json

"resolutions": {
  "jsonify": "file:./german-law-does-not-recognise-public-domain"
}
ljharb commented 7 months ago

@albertodiazdorado unfortunately it's not originally my package, and the license isn't something that can be changed without the permission of others who are unlikely to give it.

albertodiazdorado commented 7 months ago

@ljharb Thanks for the explanation, that makes sense.