You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At the moment, YAML files are loaded with require('js-yaml').load(), while JSON uses JSON.parse(). It might make sense to switch both of these to use require('yaml').parse() instead. This would have the following benefits:
yaml has better YAML spec compliance; see the YAML Test Matrix, which shows results for js-yaml as "js-yaml-json" and yaml as "js-event" and "js-json".
Safer config data loading, as the js-yaml load() supports JavaScript being encoded in YAML files. Obviously NYC also supports .js files for config, but it might still be surprising for a config loaded from a YAML file not to be guaranteed inert.
Better error reporting, in particular for JSON files -- JSON is a subset of YAML, and can be loaded as such.
Support for # comments in JSON files.
As disclosure, I'm the developer of yaml, so my opinions may be a little biased here. I'm interested, as yaml itself ends up having @istanbuljs/load-nyc-config as a dev dependency. I would be quite willing to submit a PR for this change, but thought it'd be more polite to ask first.
The text was updated successfully, but these errors were encountered:
Part of the decision when .nycrc.yml support was initially added was the ability for npm to install a yaml parser that would be shared with multiple development tools, in particular eslint and mocha both use js-yaml. Have you approached those projects with this same proposal?
Looking at the yaml package one concern I have is the inclusion of browser support, this significantly increases the install size. At quick glance of the browser code I'm confused about it using ES module syntax yet pulling in @babel/runtime. This seems like a contradiction, if a browser supports ES modules then it shouldn't need @babel/runtime?
Obviously size is not the only consideration but it does exist. Additionally although I wouldn't expect user-facing issues from switching yaml processors I would still mark such a change as 'breaking' just in case (the encoded JS as an example).
Please don't be discouraged by my response, I'm not against this idea just don't want to make such a change without scrutinizing.
At the moment, YAML files are loaded with
require('js-yaml').load()
, while JSON usesJSON.parse()
. It might make sense to switch both of these to userequire('yaml').parse()
instead. This would have the following benefits:yaml
has better YAML spec compliance; see the YAML Test Matrix, which shows results forjs-yaml
as "js-yaml-json" andyaml
as "js-event" and "js-json".js-yaml
load() supports JavaScript being encoded in YAML files. Obviously NYC also supports .js files for config, but it might still be surprising for a config loaded from a YAML file not to be guaranteed inert.# comments
in JSON files.As disclosure, I'm the developer of
yaml
, so my opinions may be a little biased here. I'm interested, asyaml
itself ends up having@istanbuljs/load-nyc-config
as a dev dependency. I would be quite willing to submit a PR for this change, but thought it'd be more polite to ask first.The text was updated successfully, but these errors were encountered: