Use user's serializer with strict: false

[ethanresnick]

Looking at #15, it seems like perhaps a better fix would've been:

function strictImpl (strict) {
  return strict === true 
    ? `throw Error('fast-redact: primitives cannot be redacted')` 
    : `return this.serialize(o)`;
}

As long as the serializer is the default JSON.stringify, I believe the behavior above would match the behavior in PR #15. But, if the user has customized the serialize function to return something other than JSON, the assumption driving #15 (that the result should always be JSON) seems invalid. It would be more consistent imo to make the redacted output always go through serialize, with strict being orthogonal.

Thoughts?

[mcollina]

Can you make an example of what will change in the output?

[ethanresnick]

If the serialize function is JSON.stringify, I don’t think this would change the output at all. But if the user is instead serializing, say, edn for all their output, then:

fastRedact({ 
  paths: [...], 
  strict: false, 
  serialize: toEDN
})(null)

would output the string nil instead of null.

edn is just an example, of course; any format with different primitive literals (eg, single quoted strings, or 0 and 1 as the representation of true/false) would be effected.

[davidmarkclements]

I agree - would you like to PR?

[ethanresnick]

Sure, will do

[ethanresnick]

Edit: thinking about this for a second more, I'm realizing it will change the existing output for non-string primitives with strict: false. E.g., redact(null) goes from null => "null" (i.e., JSON.stringify(null)).

I still think this change is for the best, though, as it separates the question of: "will I get back a string?" (Answer: always yes if serialize != false*) from "should we throw on non-objects?"

*: JSON.stringify(undefined) returns undefined, but that's the only asterisk.

[ethanresnick]

FWIW, applying the serializer to all primitive values (not just strings) is actually what @n4zukker proposed, and was decided upon here, but then that never made it into #15. I'm not sure why...

[ethanresnick]

Closing this, as it’s been addressed by #18.