-
Notifications
You must be signed in to change notification settings - Fork 12
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
Error! Unexpected token ] in "tsconfig.json" at position 666 #28
Comments
|
tsconfig files aren't technically treated like pure json files, and typically allow trailing commas and comments. Perhaps you could consider using https://github.com/dominikg/tsconfck to parse it? |
Seems like a fair request! Want to PR? |
The other option would be to replace with a regex... |
I can take a look at making a PR, sure. Seems safer and less buggy to use a parser that's built for the purpose, but if you'd rather do it with a regex that's fine too, but I'm not sure I'm up for writing it. ;) |
Actually, the fact that tsconfck resolves |
FWIW, tsconfck.parse is an async function, so it's going to mean that the rest of the functions using it will need to become async as well. So I guess it means it'll probably need a major version bump. |
I opened #36 if you want to take a quick look. I don't have time to finish it up at the moment, but I'll come back to it when I can. |
Crap... just seen the PR; yup lots of file changes. Not sure how that's going to work if they bubble up to the CLI level. Would also affect the Alias CLI. Not sure this is a good tradeoff that being the case... Thoughts? |
I think it shouldn't effect the cli, but I'll check it later. What is the alias cli? I can take a look at that too. |
It's a major component of this library:
Sure. But I don't want to rewrite the whole thing and force users to go async, just to solve some trailing commas and merging a few paths... I've posted the question here: |
It seems the asynchronous nature of the lib is due to using the async version of Node's Fs functions: https://github.com/dominikg/tsconfck/blob/60f63d821a5aefdaa8f81ab22cc7e00fbc3e973d/src/util.ts#L33 Not sure why the author chose not to use the sync version of Node's filesystem functions which I think would make it more flexible? EDIT: looks like he is doing some funky stuff with importing TS files: https://github.com/dominikg/tsconfck/blob/60f63d821a5aefdaa8f81ab22cc7e00fbc3e973d/src/util.ts#L17 |
Yeah I think you're right, it's probably not worth a breaking change to the api. Another option is to use typescript's own It's a bummer, because typescript by default adds comments to the tsconfig.json in a new project. |
Thanks for taking a look anyway. Food for thought if nothing else! And yeah, I think best to keep this sync + JS compatible. I'll reopen so this can be solved anyway. Thanks for bringing the config issue to my attention! |
Bringing in typescript wouldn't mean that it's no longer js compatible, but it would add a somewhat large dependency. But, since alias hq itself is a dev dependency, maybe it's not really a big deal. |
Re the trailing comma, this rx seems to handle it: json = json.replace(/,+(\s+[}\]])/g, '$1') Comments could be handled like so: json = json.replace(/\/\/[^"]+$/gm, '') This is just some mucking about in a scratchpad, but might be a quick win for this issue. #30 would be a bit more work. |
Maybe take a look at #37 and see if that might be a good solution? |
I just noticed you have 🦆, + 🐝 in your profile. Two of my 3 favourite creatures along with 🐙 ! Actually, add crows to that list! |
* Use typescript to parse tsconfig.json * Handle extends in config files * Fix linting errors * Bump version Co-authored-by: Dave Stewart <[email protected]> Closes #28 Closes #30
My tsconfig.json is :
Executing
alias-hq CLI
:Why? How to solve the problem?
The text was updated successfully, but these errors were encountered: