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

compile errors when typechecking svelte.config.js with moduleResolution set to nodenext #619

Closed
DetachHead opened this issue Apr 14, 2023 · 4 comments
Labels
bug Something isn't working triage Awaiting triage by a project member

Comments

@DetachHead
Copy link

DetachHead commented Apr 14, 2023

Describe the bug

when typechecking svelte.config.js with moduleResolution set to nodenext, the following compile errors occur:

$ tsc svelte.config.js --allowjs --noemit --moduleresolution nodenext
node_modules/@sveltejs/vite-plugin-svelte/dist/index.d.ts:2:41 - error TS2307: Cannot find module 'svelte/types/compiler/interfaces' or its corresponding type declarations.

2 import { CompileOptions, Warning } from 'svelte/types/compiler/interfaces';
                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sveltejs/vite-plugin-svelte/dist/index.d.ts:3:41 - error TS2307: Cannot find module 'svelte/types/compiler/interfaces' or its corresponding type declarations.

3 export { CompileOptions, Warning } from 'svelte/types/compiler/interfaces';
                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sveltejs/vite-plugin-svelte/dist/index.d.ts:4:35 - error TS2307: Cannot find module 'svelte/types/compiler/preprocess' or its corresponding type declarations.

4 import { PreprocessorGroup } from 'svelte/types/compiler/preprocess';
                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sveltejs/vite-plugin-svelte/dist/index.d.ts:5:80 - error TS2307: Cannot find module 'svelte/types/compiler/preprocess' or its corresponding type declarations.

5 export { MarkupPreprocessor, Preprocessor, PreprocessorGroup, Processed } from 'svelte/types/compiler/preprocess';
                                                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

this is also an issue in several other packages. see:

Reproduction URL

https://stackblitz.com/edit/vitejs-vite-rnb6gs?file=package.json

Reproduction

  1. wait for the dev npm script to finish running
  2. check the console output (scroll up past the errors from the vite package)

Logs

No response

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/vite-plugin-svelte: ^2.0.3 => 2.0.4 
    svelte: ^3.57.0 => 3.58.0 
    vite: ^4.3.0-beta.2 => 4.3.0-beta.5
@bluwy
Copy link
Member

bluwy commented Apr 14, 2023

When you run tsc with a jsconfig.json, tsc will not pick that config up, which it's skipLibCheck config would've fixed this. So if you run tsc --skipLibCheck in your repro, this is fixed.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 14, 2023
@DetachHead
Copy link
Author

DetachHead commented Apr 14, 2023

When you run tsc with a jsconfig.json, tsc will not pick that config up

i'm deliberately typechecking the svelte.config.js file because i want to make sure the options specified there are correct.

which it's skipLibCheck config would've fixed this. So if you run tsc --skipLibCheck in your repro, this is fixed.

skipLibCheck isn't a proper solution though, it doesn't change the fact that the types are not correct for an esm package.

digging into it further, it seems to be caused by the exports property in ./node_modules/svelte/package.json which i believe is only used by typescript when moduleResolution is set to nodenext/node16:

https://github.com/sveltejs/svelte/blob/9425f18e52477193ea04b129ccb4a1a61a86cecf/package.json#L38-L42

since svelte projects have "type": "module" in package.json making them esm packages, moduleResolution should be set to either nodenext or node16 in tsconfig.json. otherwise, typescript will incorrectly assume the project is commonjs and not raise a compile error on the following code:

// foo.js
export const foo = 1
// svelte.config.js
import { foo } from './foo' // will fail at runtime because es modules need the .js file extension

the fix is to change these imports from 'svelte/types/compiler/interfaces' to 'svelte/compiler' as specified in svelte's package.json. i've made a PR to do this: #620 but the problem is that svelte does not expose all of the types used by vite-plugin-svelte, though i'm happy to also make an upstream PR fixing that

@bluwy
Copy link
Member

bluwy commented Apr 14, 2023

i'm deliberately typechecking the svelte.config.js file because i want to make sure the options specified there are correct.

I mean the jsconfig, not the svelte config. Running tsc would only read the tsconfig.json, not jsconfig.json, because in the jsconfig.json, there's the skipLibCheck setting turned on.

skipLibCheck isn't a proper solution though, it doesn't change the fact that the types are not correct for an esm package.

I guess that's true but as you mentioned it's because svelte/compiler doesn't export it, so the rest of the ecosystem has been using the deep import.

But also, libraries can use types however they want, they have their own config and preferences. I don't think it's fair to assert your preference into other libraries, that's a reason I push towards skipLibCheck: true instead.

@DetachHead
Copy link
Author

DetachHead commented Apr 15, 2023

I mean the jsconfig, not the svelte config. Running tsc would only read the tsconfig.json, not jsconfig.json, because in the jsconfig.json, there's the skipLibCheck setting turned on.

oh right i see what you mean. that was intentional too for my minimal repro, the jsconfig wasn't needed since i was passing the relevant arguments to tsc in the command line. i forgot that file was there in https://vite.new/svelte and i should've deleted it

I guess that's true but as you mentioned it's because svelte/compiler doesn't export it, so the rest of the ecosystem has been using the deep import.

yeah this seems to be an issue with lots of packages. imo it just makes the ecosystem with node and typescript's ES modules support even more confusing for users since pretty much every package i've come across seems to not be configured to support them properly.

But also, libraries can use types however they want, they have their own config and preferences. I don't think it's fair to assert your preference into other libraries

yeah i get that many compiler options are personal preference. like i wouldn't assert my preference for the strict flag on others. however options like moduleResolution, target and lib are more objective. unlike the strictness options, they are either correct or incorrect depending on whether they match the behavior of the runtime environment.

in this case, the packages in question are ES modules (as defined in the package.json), so this moduleResolution should reflect that. (ideally, typescript should do this automatically by default - microsoft/TypeScript#51207 (comment))

that's a reason I push towards skipLibCheck: true instead.

skipLibCheck is useful to temporarily work around incorrect/conflicting type declarations in node_modules (docs), but i feel like it's often misused as a permanent way to suppress problems that can almost always can be fixed at the source.

contrary to popular belief, when packages are configured correctly, differences in strictness options rarely cause compile errors to surface in third party code (at least in my experience). this is because usually only their declaration files (*.d.ts) get type-checked instead of the source code (*.ts). if that wasn't the case, skipLibCheck wouldn't even work anyway (eg. hperrin/svelte-material-ui#532)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Awaiting triage by a project member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants