Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yarn outdated --json now prints invalid JSON #5021

Closed
jasononeil opened this issue Dec 1, 2017 · 8 comments · Fixed by #5045
Closed

yarn outdated --json now prints invalid JSON #5021

jasononeil opened this issue Dec 1, 2017 · 8 comments · Fixed by #5045
Assignees
Labels

Comments

@jasononeil
Copy link

jasononeil commented Dec 1, 2017

Do you want to request a feature or report a bug?

A bug

What is the current behavior?

With Yarn 1.3.2, when I run yarn outdated --json, I get this output, which is not valid JSON:

Jasons-MacBook-Pro:murmur jason.oneil$ yarn outdated --json
{"type":"info","data":"Color legend : \n \"<red>\"    : Major Update backward-incompatible updates \n \"<yellow>\" : Minor Update backward-compatible features \n \"<green>\"  : Patch Update backward-compatible bug fixes"}
{"type":"table","data":{"head":["Package","Current","Wanted","Latest","Package Type","URL"],"body":[["autosize-input","0.4.0","exotic","exotic","dependencies","openlistings/autosize-input#d6b638fba4bd9af9ba32e3490fd317e49fcc77aa"],["babel-eslint","7.2.3","7.2.3","8.0.2","devDependencies","https://github.com/babel/babel-eslint"],["babel-preset-jest","20.0.3","20.0.3","21.2.0","devDependencies","https://github.com/facebook/jest#readme"],["eslint","4.11.0","4.12.1","4.12.1","devDependencies","https://eslint.org"],["file-loader","0.11.2","0.11.2","1.1.5","dependencies","https://github.com/webpack/file-loader"],["flow-bin","0.50.0","0.50.0","0.59.0","devDependencies","https://github.com/flowtype/flow-bin#readme"],["jest-cli","15.1.1","15.1.1","21.2.1","devDependencies","http://facebook.github.io/jest/"],["lodash","3.10.1","3.10.1","4.17.4","dependencies","https://lodash.com/"],["moment","2.19.2","2.19.3","2.19.3","dependencies","http://momentjs.com"],["node-sass","4.5.3","4.7.2","4.7.2","dependencies","https://github.com/sass/node-sass"],["pluralize","6.0.0","6.0.0","7.0.0","dependencies","https://github.com/blakeembrey/pluralize#readme"],["postcss-loader","2.0.8","2.0.9","2.0.9","dependencies","https://github.com/postcss/postcss-loader#readme"],["react","15.6.2","15.6.2","16.2.0","dependencies","https://reactjs.org/"],["react-dom","15.5.4","15.5.4","16.2.0","dependencies","https://reactjs.org/"],["react-elm-components","1.0.0","exotic","exotic","dependencies","argshook/react-elm-components#22ae945f7c757d90fcce5277c21b202619548a4e"],["react-markdown","2.5.1","2.5.1","3.1.0","dependencies","https://github.com/rexxars/react-markdown#readme"],["react-select","1.0.0-rc.10","1.0.0-rc.10","1.1.0","dependencies","https://github.com/JedWatson/react-select#readme"],["react-select-fast-filter-options","0.2.2","exotic","exotic","dependencies","cultureamp/react-select-fast-filter-options#8c48a861da327a7a3231efa15fd84a36aadbc223"],["react-test-renderer","15.6.2","15.6.2","16.2.0","dependencies","https://reactjs.org/"],["select2","3.5.1","3.5.1","4.0.6-rc.1","dependencies","https://select2.org"],["webpack","3.8.1","3.9.1","3.9.1","dependencies","https://github.com/webpack/webpack"],["webpack-dev-server","2.9.4","2.9.5","2.9.5","devDependencies","https://github.com/webpack/webpack-dev-server"],["webpack-sources","1.0.2","1.1.0","1.1.0","dependencies","https://github.com/webpack/webpack-sources#readme"]]}}

If the current behavior is a bug, please provide the steps to reproduce.

Run yarn updated --json

What is the expected behavior?

With yarn 1.2.0, I received valid JSON as output:

Jasons-MacBook-Pro:murmur jason.oneil$ yarn outdated --json
{"type":"table","data":{"head":["Package","Current","Wanted","Latest","Package Type","URL"],"body":[["autosize-input","0.4.0","exotic","exotic","dependencies","openlistings/autosize-input#d6b638fba4bd9af9ba32e3490fd317e49fcc77aa"],["babel-eslint","7.2.3","7.2.3","8.0.2","devDependencies","https://github.com/babel/babel-eslint"],["babel-preset-jest","20.0.3","20.0.3","21.2.0","devDependencies","https://github.com/facebook/jest#readme"],["eslint","4.11.0","4.12.1","4.12.1","devDependencies","https://eslint.org"],["file-loader","0.11.2","0.11.2","1.1.5","dependencies","https://github.com/webpack/file-loader"],["flow-bin","0.50.0","0.50.0","0.59.0","devDependencies","https://github.com/flowtype/flow-bin#readme"],["jest-cli","15.1.1","15.1.1","21.2.1","devDependencies","http://facebook.github.io/jest/"],["lodash","3.10.1","3.10.1","4.17.4","dependencies","https://lodash.com/"],["moment","2.19.2","2.19.3","2.19.3","dependencies","http://momentjs.com"],["node-sass","4.5.3","4.7.2","4.7.2","dependencies","https://github.com/sass/node-sass"],["pluralize","6.0.0","6.0.0","7.0.0","dependencies","https://github.com/blakeembrey/pluralize#readme"],["postcss-loader","2.0.8","2.0.9","2.0.9","dependencies","https://github.com/postcss/postcss-loader#readme"],["react","15.6.2","15.6.2","16.2.0","dependencies","https://reactjs.org/"],["react-dom","15.5.4","15.5.4","16.2.0","dependencies","https://reactjs.org/"],["react-elm-components","1.0.0","exotic","exotic","dependencies","argshook/react-elm-components#22ae945f7c757d90fcce5277c21b202619548a4e"],["react-markdown","2.5.1","2.5.1","3.1.0","dependencies","https://github.com/rexxars/react-markdown#readme"],["react-select","1.0.0-rc.10","1.0.0-rc.10","1.1.0","dependencies","https://github.com/JedWatson/react-select#readme"],["react-select-fast-filter-options","0.2.2","exotic","exotic","dependencies","cultureamp/react-select-fast-filter-options#8c48a861da327a7a3231efa15fd84a36aadbc223"],["react-test-renderer","15.6.2","15.6.2","16.2.0","dependencies","https://reactjs.org/"],["scroll-into-view-if-needed","1.4.0","1.4.1","1.4.0","dependencies","https://stipsan.github.io/scroll-into-view-if-needed"],["select2","3.5.1","3.5.1","4.0.6-rc.1","dependencies","https://select2.org"],["webpack","3.8.1","3.9.1","3.9.1","dependencies","https://github.com/webpack/webpack"],["webpack-dev-server","2.9.4","2.9.5","2.9.5","devDependencies","https://github.com/webpack/webpack-dev-server"],["webpack-sources","1.0.2","1.1.0","1.1.0","dependencies","https://github.com/webpack/webpack-sources#readme"]]}}

Please mention your node.js, yarn and operating system version.

NodeJS: 8.6.0
Yarn: 1.3.2
OS: macOS 10.13.1

I'm not familiar with the yarn internals but I wouldn't be surprised if #4519 was the breaking change: https://github.com/yarnpkg/yarn/pull/4519/files#diff-14dc529c5b6a0f489e731b3a9aaa2c97

@ghost ghost assigned BYK Dec 1, 2017
@ghost ghost added the triaged label Dec 1, 2017
@rally25rs
Copy link
Contributor

The diff you linked does indeed seem to be what causes the additional line of output, however I think this is by design. The --json flag causes Yarn to use its json-reporter which causes each normal output line to be converted to a JSON entry. However it has always been 1 line of JSON per 1 line of output.

For example if you yarn install --json

{"type":"step","data":{"message":"Resolving packages","current":1,"total":4}}
{"type":"activityStart","data":{"id":0}}
{"type":"activityTick","data":{"id":0,"name":"[email protected]"}}
{"type":"activityTick","data":{"id":0,"name":"[email protected]"}}
{"type":"activityTick","data":{"id":0,"name":"nan@^2.3.0"}}
... etc ...
{"type":"step","data":{"message":"Building fresh packages","current":4,"total":4}}
{"type":"activitySetStart","data":{"id":1,"total":1,"workers":5}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"node-pre-gyp"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"info it worked if it ends with ok"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"node-pre-gyp"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"node-pre-gyp info using [email protected] | darwin | x64"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"[fsevents] Success: \"/Users/jvalore/Projects/yarn-test/node_modules/fsevents/lib/binding/Release/node-v57-darwin-x64/fse.node\" already installed"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"Pass --update-binary to reinstall or --build-from-source to recompile"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"node-pre-gyp"}}
{"type":"activitySetTick","data":{"id":1,"header":"fsevents","current":1,"worker":0,"message":"info ok"}}
{"type":"activitySetEnd","data":{"id":1}}

So each line is a valid JSON object. Previously, outdated just had 1 line of output, so you could parse the entire output as a single JSON object.


@jasononeil I would recommend having whatever is processing the Yarn output find the entry with "type":"table" and only process that line.

@yarnpkg/core Perhaps json-reporter should actually wrap the entire output as an array [ ... ] so that the entire output could be parsed as a single object? Though that would break any existing systems that are processing the output as individual lines... 😞 Any additional thoughts/comments on this?

@Haroenv
Copy link
Member

Haroenv commented Dec 4, 2017

I think we can just document it's json-lines format, since that's what it's always been 🤔

@jasononeil
Copy link
Author

Thanks for the clarification! The new hint in the help is perfect.

agoldis pushed a commit to agoldis/yarn that referenced this issue Dec 18, 2017
…readdir_files

* upstream/master:
  fix(cli): Write Node4+ error message to stderr (yarnpkg#5094)
  test(jest): Upgrade jest to latest available version (yarnpkg#5018)
  fix(git): Ignores irrelevant output from ls-remote (yarnpkg#5081)
  test(integration): Fix failing react-scripts test due to unexpected
warning (yarnpkg#5076)
  feat(help) Add command descriptions to commander output (yarnpkg#5033)
  ci(circle): Fix cache key setup for proper node_modules sharing
(yarnpkg#5060)
  fixed (yarnpkg#5034)
  fix(fetcher): offline mirror name collision w/ private registries and
scopes (yarnpkg#4822)
  fix(add): Make semver flags compatible with versioned requests (yarnpkg#4999)
  [yarnpkg#5021] Add help comment to --json flag (yarnpkg#5045)
  fix(git): match git dependencies by name instead of whole url
  fix(install): connectionOptions passes in localhost as its host to
prevent popup on MacOsx. (yarnpkg#5006)
  chore(eslint): ignore packages dir (yarnpkg#4963)
agoldis pushed a commit to agoldis/yarn that referenced this issue Dec 18, 2017
…readdir_files

* upstream/master:
  fix(cli): Write Node4+ error message to stderr (yarnpkg#5094)
  test(jest): Upgrade jest to latest available version (yarnpkg#5018)
  fix(git): Ignores irrelevant output from ls-remote (yarnpkg#5081)
  test(integration): Fix failing react-scripts test due to unexpected
warning (yarnpkg#5076)
  feat(help) Add command descriptions to commander output (yarnpkg#5033)
  ci(circle): Fix cache key setup for proper node_modules sharing
(yarnpkg#5060)
  fixed (yarnpkg#5034)
  fix(fetcher): offline mirror name collision w/ private registries and
scopes (yarnpkg#4822)
  fix(add): Make semver flags compatible with versioned requests (yarnpkg#4999)
  [yarnpkg#5021] Add help comment to --json flag (yarnpkg#5045)
  fix(git): match git dependencies by name instead of whole url
  fix(install): connectionOptions passes in localhost as its host to
prevent popup on MacOsx. (yarnpkg#5006)
  chore(eslint): ignore packages dir (yarnpkg#4963)
agoldis pushed a commit to agoldis/yarn that referenced this issue Dec 18, 2017
…readdir_files

* upstream/master:
  fix(cli): Write Node4+ error message to stderr (yarnpkg#5094)
  test(jest): Upgrade jest to latest available version (yarnpkg#5018)
  fix(git): Ignores irrelevant output from ls-remote (yarnpkg#5081)
  test(integration): Fix failing react-scripts test due to unexpected
warning (yarnpkg#5076)
  feat(help) Add command descriptions to commander output (yarnpkg#5033)
  ci(circle): Fix cache key setup for proper node_modules sharing
(yarnpkg#5060)
  fixed (yarnpkg#5034)
  fix(fetcher): offline mirror name collision w/ private registries and
scopes (yarnpkg#4822)
  fix(add): Make semver flags compatible with versioned requests (yarnpkg#4999)
  [yarnpkg#5021] Add help comment to --json flag (yarnpkg#5045)
  fix(git): match git dependencies by name instead of whole url
  fix(install): connectionOptions passes in localhost as its host to
prevent popup on MacOsx. (yarnpkg#5006)
  chore(eslint): ignore packages dir (yarnpkg#4963)
@GantMan
Copy link

GantMan commented Feb 23, 2018

Just a note here, quite a few tools have to do special logic supporting NPM/YARN since this isn't a single json result. I think an array would be nicer for those supporting Yarn and NPM.

Keep in mind we're passing the --json flag not the --json-lines flag. So this is a continuous curve ball for us hoping to support our users' preferences.

Bonus for devs:
Nicer even, would be if I could filter out all that noise from yarn, --json --type-filter "table" that would give me specifically what I want in a single JSON object. 100 "activityTick" objects really isn't data for most of us.

@BYK
Copy link
Member

BYK commented Feb 23, 2018

@GantMan there's a large performance tradeoff with wrapping this in an array both from Yarn's side and from the consumer side (mostly when doing streaming parsing and on large outputs). Do you think this is a huge deal?

@GantMan
Copy link

GantMan commented Feb 23, 2018

btw, I don't want to come off as a complainer. I'm happy to do any PRs of the above, should they be confirmed as acceptable. Just let me know

@GantMan
Copy link

GantMan commented Feb 23, 2018

@BYK - I can see what you're saying with streaming.

For those of us who are not taking advantage of the streaming functionality, would the cost be the same? I guess I never thought of building a stream consumer for CLI output that is fast. What percentage of people are consuming the stream? Maybe I should step up my game! :)

But the most important task for me is to provide functionality to the users (npm or yarn) based on their preference. So the small speed boost is more a "cool" than ideal. Keeping my code simple would be ideal, and therefore if there were a way to hit that speedy service in the knee, and get output that looks just like npm I'd take it. Simply to keep my LOC down. Form follows function for my need. Not sure what other needs are out there, but if streaming JSON lines were optional, that would great for everyone.

@GantMan
Copy link

GantMan commented Feb 26, 2018

just FYI: I'm workshopping parsing the output, just look at how much work is involved for me to try to get global modules and their versions. This is relatively simple over on NPM's side.

Parsing Yarn output into something useful: https://goo.gl/sUYHvQ
Parsing in NPM is one line map((dep) => dep.version, JSON.parse(x).dependencies) and it results in an object, not an array I have to search :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants