Skip to content
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

[Feature Request] expose tsconfig parser to js api #1515

Closed
dominikg opened this issue Aug 11, 2021 · 3 comments
Closed

[Feature Request] expose tsconfig parser to js api #1515

dominikg opened this issue Aug 11, 2021 · 3 comments

Comments

@dominikg
Copy link

Problem

vite 2.5 (currently in beta) uses a custom implementation to read tsconfig to pass some compilerOptions on to esbuild

The current implementation has several shortcomings:

  1. can't deal with dangling commas
  2. needs to resolve extend
  3. but does not detect extend loops

Solution

It would be great to be able to use esbuild to parse tsconfig instead
Just a simple parseTSConfig(filename) that returns an object with extends already resolved.

Alternatives considered

  1. An optional peer dependency on typescript to use typescripts own utilities.
    Unfortunately that blows up vites build and has a very complicated api.

  2. Improving the existing custom parser by patching the bugs mentioned above.
    Started it on tsconfig repo but i'd prefer esbuilds parser as it already is a vite dependency

The general usecase for esbuild

Even though esbuild has options to reference tsconfig files on some of its api, i think it would be generally useful for advanced usage like above to be able to get a resolved tsconfig object for inspection.

@evanw
Copy link
Owner

evanw commented Aug 11, 2021

Yes, this is unfortunate. I really wish the TypeScript team hadn't done this but that ship has sailed. It turns out tsconfig.json is not a JSON file.

The tsconfig.json parser in esbuild isn't general-purpose. It only parses a small subset of fields and they are parsed into esbuild's internal representation, which throws out information that it doesn't need. So I don't think it makes sense to expose.

The simplest way to do this would be to just use eval for parsing tsconfig.json. I think it's reasonable to assume that the packages you are trying to bundle aren't malicious, since they have already been installed and have already had an opportunity to run arbitrary code or do other malicious things. And I also think it's reasonable to assume that they are either well-formed tsconfig.json files or have a syntax error that eval would catch too.

The next simplest way to do this would be to use your own parser, either a third-party package like json5, something like strip-json-comments, or one you write yourself. I know you said json5 was too big but a JSON parser really doesn't have to be that big. The json5 package is around 30kb minified but I have written a JSON parser that's under 3kb so surely something could be made to be pretty small.

This functionality is independently useful to developers outside of the esbuild ecosystem and it would be hugely inefficient to require people to install esbuild (a 7mb binary executable) just to parse a tsconfig.json file. So I think something like this shouldn't be a part of esbuild itself.

@dominikg
Copy link
Author

dominikg commented Aug 12, 2021

Thank you for your detailed response and i very much agree that TypeScript should either have used plain json or a different extension + a more usable api for parsing.

I thought about eval too, but the extended tsconfig could be in a package as well and even --ignore-scripts was used during install, it could then still cause trouble. Unlikely, yes. Are there other vectors for execution also yes. But that doesn't warrant dropping the ball completely.
You could try to escape characters needed for exploitation that are not valid outside of string literals in json with sth like str.replace(/(?<!\\(?:[\\]{2})*)([();='\\^&|<>?!%]|[\.+-](?!\d))/g,'\\$1') but even then someone might find a clever way around or you've just opened yourself up to a redos vulnerability.

So it looks like the best way forward would be to provide a separate package that uses strip-json-comments and friends, uses native JSON.parse and implements extends resolution with circular reference checks on top.

@dominikg
Copy link
Author

a way forward? https://github.com/dominikg/tsconfck

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

No branches or pull requests

2 participants