-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
generated tsconfig.json #4118
generated tsconfig.json #4118
Conversation
🦋 Changeset detectedLatest commit: dd83df1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
module: 'es2020', | ||
lib: ['es2020', 'DOM'], | ||
target: 'es2020', | ||
// svelte-preprocess cannot figure out whether you have a value or a type, so tell TypeScript |
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.
Are we worried about losing these comments in the generated tsconfig file that gets included? Does tooling generally provide a way to look at the file being extended, and do we care if that becomes less readable.
Or, another way of asking it: was the only reason for the user-visible comments so that people could know which config options not to mess with?
Also sort of relatedly: Do you foresee us potentially wanting to warn or error out if the user config does override certain essential values from the generated config?
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.
I'm not particularly worried about the comments. Though I do think it's probably a good idea to warn if the user specifies incompatible values (as already happens with compilerOptions.paths
)
Is there anything similar we'd want to do with jsconfig.json? Does that support |
jsconfig.json works exactly the same way, yeah (in fact i've been testing all this with a jsconfig) |
Oh whoops, I missed in the diff that you'd updated the jsconfig in the starter templates as well to extend the same file. Never mind, nice! |
I left a few stylistic comments, but LGTM |
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
I'm getting warnings in lots of projects following this PR.
Just adding the above lines to
What's the recommended setup now? |
I'd say the recommendation is to wait for #4182 to be released. |
After this PR, I am now unable to resolve imports to asset files. It seems that the generated tsconfig defaults to only including I have resolved this issue by adding the following to my tsconfig: {
"extends": "./.svelte-kit/tsconfig.json",
"include": ["src/**/*"],
"exclude": [],
} I do think that this makes more sense than only allowing I think it would be beneficial and avoid some confusion to implement the following behaviour as default:
Edit It seems this error occurs specifically when using However, the existing behaviour of the tsconfig generation still does not solve the problem of custom paths as specified inside |
@applemonkey496 could you file a new issue about this so that we can track it? It will get lost as a comment on a closed PR |
Recommended to wait on this <sveltejs/kit#4118 (comment)>
Recommended to wait on this <sveltejs/kit#4118 (comment)>
* Added Vitest testing to about page * Added tests for basic rendering on each major acmcsuf.com page * Ran `npm run lint` * Fixed all lint errors that came along with upgrading the sveltekit deps * Added first group of tests for `/lib/ical/common` * Update CONTRIBUTING.md * Shorted npm run test to npm t in npm run all * Removed unnecessary environment comment * Added PR number to cache name * Will this fix the lint errors? * Update tsconfig.json Recommended to wait on this <sveltejs/kit#4118 (comment)> * Added Vitest testing to about page * Added tests for basic rendering on each major acmcsuf.com page * Ran `npm run lint` * Fixed all lint errors that came along with upgrading the sveltekit deps * Added first group of tests for `/lib/ical/common` * Update CONTRIBUTING.md * Shorted npm run test to npm t in npm run all * Removed unnecessary environment comment * Added PR number to cache name * Will this fix the lint errors? * Update tsconfig.json * Ran `npm run format` * Delete unused dep * Imported `LoadInput` and `LoadOutput` from '@sveltejs/kit/types/internal' (for now) * Upgraded dependencies again real quick * Path parameters are always passed as string * Resolved #337 (comment) * Resolved <#337 (comment)>
Groundwork for #647. This adds a generated
.svelte-kit/tsconfig.json
and checks that the user's config (if such exists)extends
it.This allows us to make changes like #3064 that existing apps can take advantage of (rather than only newly created apps), including the change I have planned that will address #647.
It also allows us to base certain values on the user's configuration. For example, if someone were to specify a different
lib
folder......then we can automatically generate the correct alias:
I'm not sure how much of the existing
compilerOptions
is necessary, but I figured I'd keep it there for now unless people have thoughts.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0