sveltejs / svelte-preprocess

A ✨ magical ✨ Svelte preprocessor with sensible defaults and support for: PostCSS, SCSS, Less, Stylus, Coffeescript, TypeScript, Pug and much more.
MIT License
1.76k stars 150 forks source link

`@types/babel__core` should probably be a `dependency` instead of a `devDependency` #506

Open DetachHead opened 2 years ago

DetachHead commented 2 years ago

Describe the bug when attempting to type check svelte.config.js with typescript, i get the following error:

> tsc -p tsconfig.json

node_modules/svelte-preprocess/dist/types/options.d.ts:4:55 - error TS7016: Could not find a declaration file for module '@babel/core'. 'C:/Users/amogus/IdeaProjects/network-chungus/node_modules/@babel/core/lib/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/babel__core` if it exists or add a new declaration (.d.ts) file containing `declare module '@babel/core';`

4 import type { TransformOptions as BabelOptions } from '@babel/core';
                                                        ~~~~~~~~~~~~~

because @types/babel__core is required by users downstream (if they're using typescript), i'd suggest moving it to dependencies instead of devDependencies. otherwise users will have to manually add @types/babel__core to their package.json to get rid of the errors.

To Reproduce

// svelte.config.js
import sveltePreprocess from 'svelte-preprocess'

/** @type {import('@sveltejs/kit').Config} */
const config = {
    preprocess: sveltePreprocess({ sourceMap: true }),
    compilerOptions: {
        enableSourcemap: true,
    },
}

export default config
// tsconfig.json
{
    "extends": "@tsconfig/svelte/tsconfig.json",
    "compilerOptions": {
        "allowJs": true,
        "checkJs": true,
        "skipLibCheck": false //although changing this to true would also fix the problem, i try to avoid it at all costs because it's dangerous
    }
}

Expected behavior all dependencies required for type checking downstream are included in dependencies

Information about your project:

Additional context

see https://github.com/openapi-library/OpenAPIValidators/issues/258#issuecomment-999980975 for more info

dummdidumm commented 2 years ago

Mhm this is tricky. We don't want to do that because these types are only relevant when using the babel preprocessor. But since the type definitions are static, we can't model that. Maybe we need to add comments that tell TS to ignore the type errors.

DetachHead commented 2 years ago

Maybe we need to add comments that tell TS to ignore the type errors.

unfortunately that probably won't work due to https://github.com/microsoft/TypeScript/issues/38628

but yeah i agree it's a tough situation, ideally you could have conditional dependencies based on whether you're using the babel preprocessor but i don't think npm supports such a thing