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

create-svelte: allow importing JSON modules #792

Merged
merged 2 commits into from
Mar 31, 2021
Merged

Conversation

GrygrFlzr
Copy link
Member

Since Vite supports directly importing JSON modules this should be enabled in tsconfig.json so that TypeScript or VS Code doesn't complain about it.

This doesn't need to be added to jsconfig.json.

@dummdidumm
Copy link
Member

dummdidumm commented Mar 31, 2021

Question is, if we want that option or a declare module "*.json"; in global.d.ts. Adding the tsconfig flag means JSON files will get scanned by TS to infer the type structure, which is very handy but can also be very slow for large JSON files. What's the 80% use case here?

@GrygrFlzr
Copy link
Member Author

Oh, interesting. Considering this is a Vite feature I thought Vite might have already added that type definition, but it seems to be missing from vite/client.d.ts.

Does the flag make TS scan all JSON files, or only imported ones? It seems to make sense to me that if you're explicitly importing a JSON file that you probably want to manipulate it in some way instead of treating it as a black box, so the type information would be useful compared to just getting Record<string, any>. You're right about the size though, stuff like lists of countries which was the use case that prompted this PR can be quite large.

This raises the question if it should be enabled for jsconfig.json or not. It doesn't complain, but it also doesn't show any type information if the flag isn't enabled. I guess we don't enable jsCheck anyway, so maybe we're assuming people are picking JS to opt out of types?

@dummdidumm
Copy link
Member

It only will scan the imported ones. Where the breaking point for large JSON files is, I don't know - but I guess we are talking megabytes here, not kilobytes. We had an issue over at language-tools once where intellisense was getting really slow because of it.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Let's just roll with it and maybe act once the sufficient people report slowness in the IDE

@Rich-Harris Rich-Harris merged commit dac29c5 into master Mar 31, 2021
@Rich-Harris Rich-Harris deleted the resolve-json branch March 31, 2021 16:43
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.

4 participants