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

Svelte's tooling ignores TypeScript code from previous preprocessors #1567

Open
ivanhofer opened this issue Jul 23, 2022 · 3 comments
Open
Labels
limitation Constraints of the existing architecture make this hard to fix

Comments

@ivanhofer
Copy link
Contributor

Description

Speaking of the VS Code extension and the svelte-check command, when I'm adding my own preprocessor that modifies the script tag to include additional TypeScript syntax, that part gets ignored by the tooling.

I tried svelte-check and added some debug logs to see if the custom preprocessor get's loaded. The logs show up, but If I'm adding TypeScript syntax, the tooling ignores it.

Not sure what exactly happens, but it seems that the last preprocessor doesn't use the full output from the previous preprocessors.

Proposed solution

The VS Code extension and svelte-check should take the input from the previous preprocessors.

Alternatives

Leave it like it is and don't allow custom use-cases.

Additional Information, eg. Screenshots

Here is my svelte.config.js file

import preprocess from 'svelte-preprocess'

/** @type {import('@sveltejs/kit').Config} */
const config = {
	// Consult https://github.com/sveltejs/svelte-preprocess
	// for more information about preprocessors
	preprocess: [
		{
			script: ({ content, attributes, filename }) => {
				if (filename.includes('/src/routes/') && attributes.lang === 'ts' && attributes.context === 'module') {
					console.log(`1. the custom preprocessor get's used for '${filename}'`)
					const file = filename.substring(filename.lastIndexOf('/') + 1)
					const path = file.substring(0, file.indexOf('.'))
					const typePath = `./__types/${path}`

					console.log(`2. adding type import for '${filename}'`)
					return {
						code: `
import type { Load } from '${typePath}';
console.log('this gets logged');
${content}`,
					}
				}
			},
		},
		{
			script: ({ markup, content, attributes, filename }) => {
				if (filename.includes('/src/routes/') && attributes.lang === 'ts' && attributes.context === 'module') {
					console.log(`3. the resulting output for '${filename}' is:
${content}
`)
				}
			},
		},
		preprocess(),
	],
}

export default config

The first preprocessor will add the generated type imports for the Load function.
The second preprocessor checks if the first preprocessor modified the source code
The third preprocessor is the usual svelte-preprocess that ccan handle TypeScript syntax

Here is my custom route routes/test/[id]/summary:

<script context="module" lang="ts">
	export const load: Load = ({ params }) => {
		return { props: { id: params.id } }
	}
</script>

<script lang="ts">
	export let id: number
</script>

hello {id}

When running svelte-check I get following error:
image

As you can see my custom preprocessor gets executed, but in the end I still see an error because the type import for Load is missing.

@jasonlyu123
Copy link
Member

jasonlyu123 commented Jul 24, 2022

This is primarily a duplicate of #339 although that one is specific about template preprocess, the limitation we have still applied. The type check step doesn't use the preprocess config.

It could also be argued that even if we were to fix that issue. The script preprocesses would probably still be ignored. Currently, There's no way to tell if the preprocess is intended for type checks. And we can't run all the preprocessing as the code will not be typescript anymore.

@ivanhofer
Copy link
Contributor Author

Thanks, I now understand the problem better.
So the propreocessor structure would need to be updated to be able to distinguish between "regular" preprocessors and preprocessors that make TypeScript changes? And ideally this type of preprocessor is performed in a synchronous way.

The issue you have linked (#339) is about making an asynchronous process synchronous. So I think those are two different problems that can be solved independently, right?

@jasonlyu123 jasonlyu123 added the limitation Constraints of the existing architecture make this hard to fix label Feb 6, 2023
@twag123

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
limitation Constraints of the existing architecture make this hard to fix
Projects
None yet
Development

No branches or pull requests

3 participants