fix(server): use flatted for json.stringify #3220
Conversation
lib/server.js
Outdated
| @@ -8,6 +8,8 @@ const spawn = require('child_process').spawn | |||
| const tmp = require('tmp') | |||
| const fs = require('fs') | |||
| const path = require('path') | |||
| const {stringify} = require('flatted/cjs') | |||
There was a problem hiding this comment.
Would you mind to wrap this dependency into JSONUtils and add unit tests for it, something like we have with sha1 calculation ?
I like this kind of wrapping because of two reasons:
- dependencies sometimes change theirs APIs and behaviour - with wrapper covered by unit tests we're able to figure it out very early
- it's easier to remove dependency in future when it's used in single place
test/unit/utils/json-utils.spec.js
Outdated
|
|
||
| const JsonUtils = require('../../../lib/utils/json-utils') | ||
|
|
||
| describe('Utils', () => { |
There was a problem hiding this comment.
It would be nice to add one test that covers circular structure issue.
Remove json3 dep and use, probably was needed for older node code. Fixes karma-runner#3215
|
@johnjbarton I see this made it to master, any idea of when a release will be cut with this feature? The circular json error this fixes is blocking angular 7 workflows from using karma |
|
As a temporary solution it's possible to set dependency as github branch link. This article may be useful in this context : https://medium.com/@jonchurch/use-github-branch-as-dependency-in-package-json-5eb609c81f1a |
|
@lusarz thanks for the link, that is definitely helpful to unblock some of my side projects. I'm hesitant to use that method for a prod system however, EDIT: Just saw v3.1.3, thanks you guys! |
Remove json3 dep and use, probably was needed for older node code.
Fixes #3215