search

raineorshine / npm-check-updates

Find newer versions of package dependencies than what your package.json allows
Other
9.44k stars 329 forks source link

JSON5 support? #83

Closed mathieumg closed 8 years ago

mathieumg commented 9 years ago

Is there JSON5 support planned? If not, would you be open to a PR that adds it? We use https://github.com/rlidwka/yapm which supports package.json5 files. The change would be simple enough, you'd need to be using https://github.com/rlidwka/jju directly (you're somewhat already using it if the parse fails, see below) to manipulate the package.json5 file, you'd then only need to lookup for a package.json5 first (or provide a switch to specify the filename, or something like that).

Regarding jju, you're currently using https://github.com/tjunnone/npm-check-updates/blob/master/lib/npm-check-updates.js#L11 which uses https://github.com/npm/read-package-json/blob/master/read-json.js#L12 which in turn uses https://github.com/smikes/json-parse-helpfulerror/blob/master/index.js#L3 when the parse fails. jju is able to manipulate and save to a JSON5 file even when there are comments or other JSON5 specifics in it, see the live demo: https://rlidwka.github.io/jju/editor.html

raineorshine commented 9 years ago

This sounds like a great idea! If the json reading can be abstracted away from the logic, it shouldn’t be a problem. I’d be happy to accept a pull request and provide advisement on any questions.

I have been refactoring a lot lately, in preparation for v2, but it shouldn’t be changing too much now.

On Tue, Jun 9, 2015 at 11:08 AM, Mathieu M-Gosselin < notifications@github.com> wrote:

Is there JSON5 support planned? If not, would you be open to a PR that adds it? We use https://github.com/rlidwka/yapm which supports package.json5 files. The change would be simple enough, you'd need to be using https://github.com/rlidwka/jju directly (you're somewhat already using it if the parse fails, see below) to manipulate the package.json5 file, you'd then only need to lookup for a package.json5 first (or provide a switch to specify the filename, or something like that).

Regarding jju, you're currently using https://github.com/tjunnone/npm-check-updates/blob/master/lib/npm-check-updates.js#L11 which uses https://github.com/npm/read-package-json/blob/master/read-json.js#L12 which in turn uses https://github.com/smikes/json-parse-helpfulerror/blob/master/index.js#L3 when the parse fails. jju is able to manipulate and save to a JSON5 file even when there are comments or other JSON5 specifics in it, see the live demo: https://rlidwka.github.io/jju/editor.html

— Reply to this email directly or view it on GitHub https://github.com/tjunnone/npm-check-updates/issues/83.

raineorshine commented 9 years ago

read-package-json removed

mathieumg commented 9 years ago

Oh, I missed that it wasn't used. Yeah, the JSON reading would be abstracted, I'll try to tackle this soon when I get a couple spare seconds! :grin:

mathieumg commented 9 years ago

I'm working on integrating this, but I noticed that master is only reporting a few available updates compared to alpha 9, do you want me to open a separate issue for that?

2.0.0 - Alpha 9:
"async" can be updated from ^1.0.0 to ^1.2.1 (Installed: 1.0.0, Latest: 1.2.1)
"babel-core" can be updated from ^5.4.7 to ^5.5.8 (Installed: 5.4.7, Latest: 5.5.8)
"body-parser" can be updated from ^1.12.4 to ^1.13.1 (Installed: 1.12.4, Latest: 1.13.1)
"compression" can be updated from 1.4.4 to 1.5.0 (Installed: 1.4.4, Latest: 1.5.0)
"csurf" can be updated from ^1.8.2 to ^1.8.3 (Installed: 1.8.2, Latest: 1.8.3)
"express-session" can be updated from ^1.11.2 to ^1.11.3 (Installed: 1.11.2, Latest: 1.11.3)
"lodash" can be updated from ^3.9.2 to ^3.9.3 (Installed: 3.9.2, Latest: 3.9.3)
"morgan" can be updated from ^1.5.3 to ^1.6.0 (Installed: 1.5.3, Latest: 1.6.0)
"serve-favicon" can be updated from ^2.2.1 to ^2.3.0 (Installed: 2.2.1, Latest: 2.3.0)
"validator" can be updated from ^3.40.0 to ^3.40.1 (Installed: 3.40.0, Latest: 3.40.1)
"yargs" can be updated from ^3.10.0 to ^3.11.0 (Installed: 3.10.0, Latest: 3.11.0)
"babel-loader" can be updated from ^5.1.3 to ^5.1.4 (Installed: 5.1.3, Latest: 5.1.4)
"bootstrap" can be updated from ^3.3.4 to ^3.3.5 (Installed: 3.3.4, Latest: 3.3.5)
"css-loader" can be updated from ^0.14.4 to ^0.15.1 (Installed: 0.14.5, Latest: 0.15.1)
"extract-text-webpack-plugin" can be updated from ^0.8.1 to ^0.8.2 (Installed: 0.8.1, Latest: 0.8.2)
"react-bootstrap" can be updated from ^0.22.6 to ^0.23.4 (Installed: 0.22.6, Latest: 0.23.4)
"react-a11y" can be updated from ^0.1.1 to ^0.2.6 (Installed: 0.1.1, Latest: 0.2.6)
"react-fa" can be updated from ^3.2.0 to ^3.3.1 (Installed: 3.2.0, Latest: 3.3.1)
"react-typeahead" can be updated from ^1.0.5 to ^1.0.8 (Installed: 1.0.5, Latest: 1.0.8)
"react-widgets" can be updated from ^2.7.0 to ^2.7.1 (Installed: 2.7.0, Latest: 2.7.1)
"spin.js" can be updated from ^2.1.0 to ^2.3.1 (Installed: 2.1.0, Latest: 2.3.1)
"webpack" can be updated from ~1.9.10 to ~1.9.11 (Installed: 1.9.10, Latest: 1.9.11)
master - 3cf3d8a5364647188427dbefdbf3f42250eacded:
"compression" can be updated from 1.4.4 to 1.5.0 (Installed: 1.4.4, Latest: 1.5.0)
"css-loader" can be updated from ^0.14.4 to ^0.15.1 (Installed: 0.14.5, Latest: 0.15.1)
"react-bootstrap" can be updated from ^0.22.6 to ^0.23.4 (Installed: 0.22.6, Latest: 0.23.4)
"react-a11y" can be updated from ^0.1.1 to ^0.2.6 (Installed: 0.1.1, Latest: 0.2.6)
package.json :
{
    "name": "example",
    "version": "0.0.1",
    "devDependencies": {
        "accounting": "^0.4.1",
        "autoprefixer-loader": "^2.0.0",
        "babel-loader": "^5.1.3",
        "bootstrap": "^3.3.4",
        "bower-webpack-plugin": "^0.1.8",
        "codemirror": "^5.3.0",
        "css-loader": "^0.14.4",
        "exports-loader": "^0.6.2",
        "expose-loader": "^0.7.0",
        "extract-text-webpack-plugin": "^0.8.1",
        "file-loader": "^0.8.4",
        "i18next-client": "^1.9.0",
        "i18next-static-analysis": "^0.0.2",
        "jquery": "^2.1.4",
        "jquery-form": "^3.50.0",
        "json-loader": "^0.5.2",
        "json5-loader": "^0.6.0",
        "less": "^2.5.1",
        "less-loader": "^2.2.0",
        "moment": "^2.10.3",
        "react-bootstrap": "^0.22.6",
        "react-a11y": "^0.1.1",
        "react-fa": "^3.2.0",
        "react-hot-loader": "^1.2.7",
        "react-typeahead": "^1.0.5",
        "react-widgets": "^2.7.0",
        "react-widgets-moment-localizer": "^1.0.1",
        "spin.js": "^2.1.0",
        "style-loader": "^0.12.3",
        "summernote": "0.6.7",
        "superagent": "^1.2.0",
        "url-loader": "^0.5.6",
        "webpack": "~1.9.10",
        "webpack-dev-server": "^1.9.0",
        "world-countries": "^1.7.3",
        "world-currencies": "0.0.5"
    },
    "dependencies": {
        "async": "^1.0.0",
        "babel-core": "^5.4.7",
        "body-parser": "^1.12.4",
        "compression": "1.4.4",
        "connect-flash": "0.1.x",
        "connect-html-minifier": "0.1.0",
        "connect-redis": "2.x",
        "cookie-parser": "^1.3.5",
        "csurf": "^1.8.2",
        "ewait": "~0.2.1",
        "express": "4.12.x",
        "express-session": "^1.11.2",
        "forwarded-for": "^1.0.0",
        "i18next": "^1.9.0",
        "json5": "^0.4.0",
        "lodash": "^3.9.2",
        "mailchimp-api": "^2.0.7",
        "maxmind": "^0.5.5",
        "morgan": "^1.5.3",
        "multer": "^0.1.8",
        "node-uuid": "^1.4.3",
        "numeral": "^1.5.3",
        "passport": "^0.2.2",
        "passport-facebook": "^2.0.0",
        "passport-google-oauth": "^0.2.0",
        "passport-linkedin": "^0.1.3",
        "passport-local": "^1.0.0",
        "react": "^0.13.3",
        "serve-favicon": "^2.2.1",
        "swig": "^1.4.2",
        "validator": "^3.40.0",
        "yargs": "^3.10.0"
    }
}
raineorshine commented 9 years ago

The semantics in master are slightly different than in previous versions, though the installed versions end up being the same. For example, if the async dependency is listed as ^1.0.0, and the latest version is 1.2.1, then the dependency does not need to be rewritten since 1.2.1 is included within the ^1.0.0 range. That is, a normal npm update will install 1.2.1. npm-check-updates is intended to only fill in functionality that is not present in npm, which was confused before.

I could potentially add a flag to control this, but I’d have to be convinced. I’m concerned that people who want the old-style behavior just don't know about npm update.

On Thu, Jun 18, 2015 at 9:27 AM, Mathieu M-Gosselin < notifications@github.com> wrote:

I'm working on integrating this, but I noticed that master is only reporting a few available updates compared to alpha 9, do you want me to open a separate issue for that? 2.0.0 - Alpha 9:

"async" can be updated from ^1.0.0 to ^1.2.1 (Installed: 1.0.0, Latest: 1.2.1) "babel-core" can be updated from ^5.4.7 to ^5.5.8 (Installed: 5.4.7, Latest: 5.5.8) "body-parser" can be updated from ^1.12.4 to ^1.13.1 (Installed: 1.12.4, Latest: 1.13.1) "compression" can be updated from 1.4.4 to 1.5.0 (Installed: 1.4.4, Latest: 1.5.0) "csurf" can be updated from ^1.8.2 to ^1.8.3 (Installed: 1.8.2, Latest: 1.8.3) "express-session" can be updated from ^1.11.2 to ^1.11.3 (Installed: 1.11.2, Latest: 1.11.3) "lodash" can be updated from ^3.9.2 to ^3.9.3 (Installed: 3.9.2, Latest: 3.9.3) "morgan" can be updated from ^1.5.3 to ^1.6.0 (Installed: 1.5.3, Latest: 1.6.0) "serve-favicon" can be updated from ^2.2.1 to ^2.3.0 (Installed: 2.2.1, Latest: 2.3.0) "validator" can be updated from ^3.40.0 to ^3.40.1 (Installed: 3.40.0, Latest: 3.40.1) "yargs" can be updated from ^3.10.0 to ^3.11.0 (Installed: 3.10.0, Latest: 3.11.0) "babel-loader" can be updated from ^5.1.3 to ^5.1.4 (Installed: 5.1.3, Latest: 5.1.4) "bootstrap" can be updated from ^3.3.4 to ^3.3.5 (Installed: 3.3.4, Latest: 3.3.5) "css-loader" can be updated from ^0.14.4 to ^0.15.1 (Installed: 0.14.5, Latest: 0.15.1) "extract-text-webpack-plugin" can be updated from ^0.8.1 to ^0.8.2 (Installed: 0.8.1, Latest: 0.8.2) "react-bootstrap" can be updated from ^0.22.6 to ^0.23.4 (Installed: 0.22.6, Latest: 0.23.4) "react-a11y" can be updated from ^0.1.1 to ^0.2.6 (Installed: 0.1.1, Latest: 0.2.6) "react-fa" can be updated from ^3.2.0 to ^3.3.1 (Installed: 3.2.0, Latest: 3.3.1) "react-typeahead" can be updated from ^1.0.5 to ^1.0.8 (Installed: 1.0.5, Latest: 1.0.8) "react-widgets" can be updated from ^2.7.0 to ^2.7.1 (Installed: 2.7.0, Latest: 2.7.1) "spin.js" can be updated from ^2.1.0 to ^2.3.1 (Installed: 2.1.0, Latest: 2.3.1) "webpack" can be updated from ~1.9.10 to ~1.9.11 (Installed: 1.9.10, Latest: 1.9.11)

master - :

"compression" can be updated from 1.4.4 to 1.5.0 (Installed: 1.4.4, Latest: 1.5.0) "css-loader" can be updated from ^0.14.4 to ^0.15.1 (Installed: 0.14.5, Latest: 0.15.1) "react-bootstrap" can be updated from ^0.22.6 to ^0.23.4 (Installed: 0.22.6, Latest: 0.23.4) "react-a11y" can be updated from ^0.1.1 to ^0.2.6 (Installed: 0.1.1, Latest: 0.2.6) ``

package.json :

{ "name": "example", "version": "0.0.1", "devDependencies": { "accounting": "^0.4.1", "autoprefixer-loader": "^2.0.0", "babel-loader": "^5.1.3", "bootstrap": "^3.3.4", "bower-webpack-plugin": "^0.1.8", "codemirror": "^5.3.0", "css-loader": "^0.14.4", "exports-loader": "^0.6.2", "expose-loader": "^0.7.0", "extract-text-webpack-plugin": "^0.8.1", "file-loader": "^0.8.4", "i18next-client": "^1.9.0", "i18next-static-analysis": "^0.0.2", "jquery": "^2.1.4", "jquery-form": "^3.50.0", "json-loader": "^0.5.2", "json5-loader": "^0.6.0", "less": "^2.5.1", "less-loader": "^2.2.0", "moment": "^2.10.3", "react-bootstrap": "^0.22.6", "react-a11y": "^0.1.1", "react-fa": "^3.2.0", "react-hot-loader": "^1.2.7", "react-typeahead": "^1.0.5", "react-widgets": "^2.7.0", "react-widgets-moment-localizer": "^1.0.1", "spin.js": "^2.1.0", "style-loader": "^0.12.3", "summernote": "0.6.7", "superagent": "^1.2.0", "url-loader": "^0.5.6", "webpack": "~1.9.10", "webpack-dev-server": "^1.9.0", "world-countries": "^1.7.3", "world-currencies": "0.0.5" }, "dependencies": { "async": "^1.0.0", "babel-core": "^5.4.7", "body-parser": "^1.12.4", "compression": "1.4.4", "connect-flash": "0.1.x", "connect-html-minifier": "0.1.0", "connect-redis": "2.x", "cookie-parser": "^1.3.5", "csurf": "^1.8.2", "ewait": "~0.2.1", "express": "4.12.x", "express-session": "^1.11.2", "forwarded-for": "^1.0.0", "i18next": "^1.9.0", "json5": "^0.4.0", "lodash": "^3.9.2", "mailchimp-api": "^2.0.7", "maxmind": "^0.5.5", "morgan": "^1.5.3", "multer": "^0.1.8", "node-uuid": "^1.4.3", "numeral": "^1.5.3", "passport": "^0.2.2", "passport-facebook": "^2.0.0", "passport-google-oauth": "^0.2.0", "passport-linkedin": "^0.1.3", "passport-local": "^1.0.0", "react": "^0.13.3", "serve-favicon": "^2.2.1", "swig": "^1.4.2", "validator": "^3.40.0", "yargs": "^3.10.0" } }

— Reply to this email directly or view it on GitHub https://github.com/tjunnone/npm-check-updates/issues/83#issuecomment-113191907 .

mathieumg commented 9 years ago

Ohh, fair enough, if it's well documented!

I'd still welcome a flag to enable the old behavior, even if semver would install 1.30 with either of ^1.0.0 and ^1.30.0, I like the explicitness of the latter. A possible use case for this is when you're curious about what dependencies in your package have updates, breaking or not.

raineorshine commented 9 years ago

Thanks, I’ll consider it! Check out “npm outdated —depth=0” if you haven’t yet; does a similar thing.

On Fri, Jun 19, 2015 at 10:15 AM, Mathieu M-Gosselin < notifications@github.com> wrote:

Ohh, fair enough, if it's well documented!

I'd still welcome a flag to enable the old-behavior, even if semver would install 1.30 with either of ^1.0.0 and ^1.30.0, I like the explicitness of the latter. A possible use case for this is when you're curious about what dependencies in your package have updates, breaking or not.

— Reply to this email directly or view it on GitHub https://github.com/tjunnone/npm-check-updates/issues/83#issuecomment-113561976 .

mathieumg commented 9 years ago

Yeah I knew that command, thanks for considering! :smile:

etiktin commented 9 years ago

In PR #132, I added the json-parse-helpfulerror module which uses jju. So while I was working on it, I was thinking about going all in and adding support for package.json5. I didn't because I found some issues:

It's definitely possible to support this, but I think the amount of work needed, the unknown state of yapm and the limitations such a solution will inflict (e.g. we will need to replicate modules such as closest-package, and make them support the other formats) means we shouldn't do it.

What do you guys think?

raineorshine commented 9 years ago

Thanks for the breakdown, Eran. It sounds like significant work for minimal gain at the moment.

mathieumg commented 9 years ago

Thanks @etiktin, this is consistent with my previous findings and what I mentioned in https://github.com/tjunnone/npm-check-updates/issues/94#issuecomment-123826843

etiktin commented 9 years ago

I also like the idea of having comments in package.json, but it seems that using other formats is just too complicated and fragile. I think a simpler way to go, would be to create a separate file to hold the meta data (comments) and keep package.json as is, so it will work with the existing ecosystem.

For example, lets say I have the following package.json:

{
  "name": "ncu-tester",
  "version": "1.0.0",
  "description": "tester",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "bindings": "^1.1.0",
    "express": "^3.21.0",
    "ref": "^0.3.5"
  }
}

My package.json.meta could be something like this (using a CSON syntax):

dependencies: 'This is a single line comment'
dependencies.bindings: 'Another single line comment'
dependencies.express: '''
    This is a multi line comment
    Second line
    Third line
    '''

Then in your favorite text editor, you will have a smart plugin that can show you the preview of the meta applied on top of the json (kind of like the editors that show you a webpage preview next to the source files your editing):

{
  "name": "ncu-tester",
  "version": "1.0.0",
  "description": "tester",
  "main": "index.js",
  "license": "MIT",

  // This is a single line comment
  "dependencies": {

    // Another single line comment
    "bindings": "^1.1.0",

    /* 
    This is a multi line comment
    Second line
    Third line
    */
    "express": "^3.21.0",
    "ref": "^0.3.5"
  }
}

The plugin might even be smart enough to allow you to edit it inline and when you save, comments will be written to the meta file and keys and values will be written to the json file. If we want to support multiple different formats for the meta file, we can agree on a convention that you have a field in the json that says what meta file to use (e.g. "metaFile": "packageMeta.cson"). The relationship between the json and it's meta, is somewhat similar to the relationship between html and css.

This way of doing things seems more sustainable to me.

mathieumg commented 9 years ago

@etiktin I agree this is an interesting take and an innovative way to approach the issue, however it implicates editor-dependent tooling. :( An upside is that it could probably be used for many other projects who have strict JSON as their configuration file as well.

It would need to handle cases where a/many valid JSON line is/are commented out, as well. In such cases it would need to remove the line(s) from the JSON file and add it/them as a comment to the appropriate sibling node in the meta file, and vice-versa. It would need to handle these cases when there are no siblings as well, possibly due to the same operation.

There's no denying it would be much more sustainable than forking or monkey patching the package manager.

raineorshine commented 8 years ago

Closing for cleanup purposes and adding the revive-me label. Feel free to re-open if you are interested in working on a pull request.

mathieumg commented 8 years ago

I was waiting on https://github.com/hughsk/closest-package/pull/2

I guess I'll just create my fork.

raineorshine commented 8 years ago

Sure. Let me know if you're actively working on it and I'll re-open.