Skip to content

Make use of new safe-stable-stringify 2.x features #134

Merged
DABH merged 1 commit intowinstonjs:masterfrom
mastermatt:upgrade-safe-stable-stringify
Feb 12, 2022
Merged

Make use of new safe-stable-stringify 2.x features #134
DABH merged 1 commit intowinstonjs:masterfrom
mastermatt:upgrade-safe-stable-stringify

Conversation

@mastermatt
Copy link
Copy Markdown
Contributor

https://github.com/BridgeAR/safe-stable-stringify/releases

  • Allow opts to configure the lib.
  • Update types for new opts.
  • Add tests to fill in coverage for json.js.

NB. The Buffer.toString('base64') conversion was removed. Buffer implements toJSON so this block of code was not being executed anyway, as the replacer is called with the result of toJSON. Base64 was never being returned.
https://nodejs.org/api/buffer.html#buftojson

@mastermatt mastermatt force-pushed the upgrade-safe-stable-stringify branch from bdb7743 to c5df278 Compare January 27, 2022 19:47
@DABH
Copy link
Copy Markdown
Contributor

DABH commented Feb 12, 2022

@mastermatt I'm taking a look at this now. I just merged a dependabot PR that upgraded to 2.x so could you please rebase or fix merge conflicts here and I can look at the changes you have besides the version bump? Thanks!

@mastermatt mastermatt force-pushed the upgrade-safe-stable-stringify branch from c5df278 to f3a716c Compare February 12, 2022 16:13
@mastermatt mastermatt changed the title Upgrade safe-stable-stringify lib to 2.x Make use of new safe-stable-stringify 2.x features Feb 12, 2022
@mastermatt mastermatt force-pushed the upgrade-safe-stable-stringify branch from f3a716c to 6dd81e7 Compare February 12, 2022 16:17
@mastermatt
Copy link
Copy Markdown
Contributor Author

@DABH done.

This commit originally included an upgrade of the lib itself from 1.x to 2.x.
After a Dependabot PR was merged in to do the upgrade, this commit was rebased to only include the changes that take advantage of the new features.
https://github.com/BridgeAR/safe-stable-stringify/releases

----

- Allow `opts` to configure the lib.
- Update types for new opts.
- Add tests to fill in coverage for json.js.

NB. The `Buffer.toString('base64')` conversion was removed. Buffer implements `toJSON` so this block of code was not being executed anyway, as the replacer is called with the result of `toJSON`. Base64 was never being returned.
https://nodejs.org/api/buffer.html#buftojson
@mastermatt mastermatt force-pushed the upgrade-safe-stable-stringify branch from 6dd81e7 to fef36ef Compare February 12, 2022 16:26
Copy link
Copy Markdown
Contributor

@DABH DABH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for adding tests!

@DABH DABH merged commit e3a8000 into winstonjs:master Feb 12, 2022
@mastermatt mastermatt deleted the upgrade-safe-stable-stringify branch February 14, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants