New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: strip dangling comma #31
Conversation
built a separate package with with support for dangling comma, extends and parsing with ts api: https://github.com/dominikg/tsconfck |
What do you think about using regex like strip-json-trailing-commas did? It is simple and works in all cases. |
* | ||
* implementation heavily inspired by strip-json-comments | ||
*/ | ||
function stripDanglingComma (jsonString: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function stripDanglingComma (jsonString: string) { | |
function stripDanglingCommas (jsonString: string) { |
@@ -185,7 +185,7 @@ export function readFileSync (filename: string): any { | |||
* Parse `tsconfig.json` file. | |||
*/ | |||
export function parse (contents: string, filename: string) { | |||
const data = stripComments(stripBom(contents)) | |||
const data = stripDanglingComma(stripComments(stripBom(contents))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const data = stripDanglingComma(stripComments(stripBom(contents))) | |
const data = stripDanglingCommas(stripComments(stripBom(contents))) |
I don't like that idea. a regex to only match dangling commas outside of string values is a lot more involved than the iterative approach taken here. but at the same time i'm no longer invested in this PR, see my comment above mentioning tsconfck. if the maintainers of tsconfig ever turn up here again it's up to them |
fixes #30
note that package.json needs serious updates.
in it's current state build fails with
the output of
npm audit
ain't pretty either.I suggest a major update on all fronts and cutting 8.0