Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.35%. Comparing base (f86e911) to head (902aaad).
Report is 8 commits behind head on main.

@@           Coverage Diff           @@
##             main    #5876   +/-   ##
=======================================
  Coverage   96.34%   96.35%           
=======================================
  Files         479      479           
  Lines       38674    38673    -1     
  Branches     5631     5630    -1     
=======================================
+ Hits        37260    37262    +2     
+ Misses       1370     1367    -3     
  Partials       44       44           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This changes the result of stringify({ foo: new Boolean(true) }) to foo: {} instead of foo: true. I don't think this matches the expectation of the users.

I think we should fix represent handler of bool type instead. I guess something like Boolean(object) ? "true" : "false" should fix the issue.

This changes the result of stringify({ foo: new Boolean(true) }) to foo: {} instead of foo: true. I don't think this matches the expectation of the users.

I think we should fix represent handler of bool type instead. I guess something like Boolean(object) ? "true" : "false" should fix the issue.

The problem is that this special treats the Boolean object instance. We have the same problem with

stringify({ foo: new String("foo") })

which returns

foo:
  '0': f
  '1': o
  '2': o

and

stringify({ foo: new Number(1) })

which returns

foo: {}

Tbh, I don't know if we should special treat these cases as they should be discouraged anyway.

The problem is that this special treats the Boolean object instance. We have the same problem with

stringify({ foo: new String("foo") })

which returns

foo:
  '0': f
  '1': o
  '2': o

and

stringify({ foo: new Number(1) })

which returns

foo: {}

Those examples also seem bugs to me.

std/yaml is originally ported from js-yaml and js-yaml handles those cases as foo: foo and foo: 1.

import * as yaml from "npm:js-yaml";

console.log(yaml.dump({ foo: new String("foo") }));
console.log(yaml.dump({ foo: new Number(1) }));

Looks like those handlings of new String and new Number are regression in recent versions. It used to be handled as string and number:

import { stringify } from "https://deno.land/[email protected]/yaml/mod.ts";

console.log(stringify({ foo: new String("foo") }));
console.log(stringify({ foo: new Number(1) }));

This prints:

foo: foo
foo: 1

Closing in favour of #5894