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

Allow <script lang='typescript'> without an explicit pre-processor #4797

Closed
orta opened this issue May 7, 2020 · 8 comments
Closed

Allow <script lang='typescript'> without an explicit pre-processor #4797

orta opened this issue May 7, 2020 · 8 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@orta
Copy link
Contributor

orta commented May 7, 2020

Describe the bug

We can reduce the number of tools people need to set up by adding very minimal support for TypeScript parsing of the JS. This can be done in the same way that Danger does it:

To Reproduce

<script lang="typescript">
	let name:string = 'world';
</script>

<h1>Hello {name}!</h1>

Expected behavior
This would pass, assuming TypeScript was in my dependencies

Stacktraces
n/a

Severity
Nice to have - a part of #4518

@Conduitry
Copy link
Member

I don't like this for a few reasons. One is that the core compile() function is pure. It does no file system interactions, and makes no use of Node-only libraries. This permits a nice separation of concerns between compilation and other things which are the responsibility of the bundler and the bundler's Svelte plugin. This also lets the compiler run without issue in the browser for the REPL.

From a more technical perspective: Do Babel and TypeScript have synchronous modes of operation? The Svelte compiler is currently synchronous, and one of the reasons that preprocessing was split out into its own thing was that async-only operations abound in the various transpilers people would want to use on components.

@orta
Copy link
Contributor Author

orta commented May 7, 2020

For the first point, that's totally reasonable, you all know the internals better than I do. There could definitely be other avenues I don't know to explore.

Second: Yeah, in the Danger code above they are both synchronous

@dreitzner
Copy link
Contributor

@Conduitry maybe we could think about a auto detect feature inside the preprocess for common use cases?

@benmccann
Copy link
Member

What does "minimal support" mean? Are there things that would be explicitly unsupported?

I'd love better TypeScript support, but I tend to think that building in support for pre-processing TypeScript and not other things like sass, less, etc. would be a bit inconsistent

One thing I wonder about building into sveltejs though is compilation of templates to TypeScript (e.g. like tsx does). That would make a lot more sense to do in core than trying to build it into an external library

@orta
Copy link
Contributor Author

orta commented May 16, 2020

For me, minimal is that if svelte see's typescript lang tags it runs transpileModuleon the code and replaces it with the result JS on before doing any work. That's all, the goal being to avoid feature creep and allow an exception to the norm of the 'use a preprocessor lib' advice.

It really could just be the "typescript" module, which would simplify the code to:

export const typescriptify = (content: string): string => {
  try {
    const ts = require("typescript") 
    const compilerOptions = { ... }
    const result = ts.transpileModule(content, sanitizeTSConfig(compilerOptions))
    return result.outputText
  } catch () {
    console.error("You need TypeScript installed to use TypeScript script templates in svelte")
    process.exitCode = 1
    process.exit() // etc
 }
}

For me, doing something like:

One thing I wonder about building into sveltejs though is compilation of templates to TypeScript (e.g. like tsx does). That would make a lot more sense to do in core than trying to build it into an external library

Is pretty heavy work (as seen in svelte2tsx) which puts a lot of support and tooling inside the svelte compiler

@matthewmueller
Copy link

matthewmueller commented May 24, 2020

Adding my 2 cents:

  • I'd love to see this work without a svelte.config.js file in each project. I'm personally quite tired of all the configuration clutter. Allowing it to be defined in package.json (similar to how prettier supports both) would solve this for me.

  • Does LSP need to be bound to the project's build system? This should probably be the case as drift can occur in both the Typescript compiler and the Svelte compiler, but I do like how Typescript allows you to just write in Typescript, then add a tsconfig.json when you're ready.

  • It appears to only work with the Svelte Beta LSP. I'm not sure if this is intentional, but I didn't find that documented.

@orta
Copy link
Contributor Author

orta commented May 24, 2020

This is issue about moving enough into the compiler so that you wouldn't need the preprocessor config for just TS - yep. Re: config stuff, that's a good call for another issue.

The proposal doesn't touch the editor integration (which is what the LSP handles) they're not connected other than the LSP uses the svelte compiler under the hood. The LSP wouldn't have any code changes if/when this change happens.

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Jun 8, 2020
@orta
Copy link
Contributor Author

orta commented Jul 4, 2020

I brought up my opinion in a recent maintainers meeting - I've been shifting to the side of this is not necessary. I'm going to close the issue.

When you have svelte-preprocess set up by without a config file, TypeScript <script> tags are supported with no config file. This is enough for me to not think it's worth the extra compiler support, it is basically already there.

@orta orta closed this as completed Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

5 participants