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

Reimagining svelte2tsx #1077

Closed
dummdidumm opened this issue Jun 27, 2021 · 3 comments
Closed

Reimagining svelte2tsx #1077

dummdidumm opened this issue Jun 27, 2021 · 3 comments

Comments

@dummdidumm
Copy link
Member

dummdidumm commented Jun 27, 2021

svelte2tsx has come quite a long way and is successfully representing some rather advanced use cases of Svelte in a TSX-compatible way. But there are significant drawbacks that continue to pile up:

  • Code that is needed to create the output and the output itself is hard to read because of all the workarounds we have to use, especially around shadowed variables and control flow
  • The way we use MagicString is hard to read, and has some limitations around good source mapping, especially when comes to ranges (length is often short by one)
  • Any feature that is not directly translateable to TSX is hard to implement and we need to work around it with lots of lambda functions
  • The generated TSX needs some patches of the compiler options to work with the Svelte JSX namespace, which can interfere with other (f.e. React) projects (Svelte for VS Code extension crushes react project #1149)

What I envision is going away from a somewhat TSX-compliant structure and reimagine the translation more in line with our needs. One possible change is to no longer try to keep the HTML structure and instead focus on the control flow structure.

Given this input:

{#if hello}
    <div>
    {#await hello.foo then y}
        {y}
    {/await}
    </div>
{/if}

We no longer do this:

<>{(hello) ? <>
    <div>
    {() => {/*Ωignore_startΩ*/if(((hello))) {/*Ωignore_endΩ*/let _$$p = (hello.foo); __sveltets_1_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<>
        {y}
    </>})}}}
    </div>
</> : <></>}</>

Instead we do this:

if (hello) {
  const div = createElement('div');
  const y = await hello.foo;
  y;
}

What this means is that we no longer try to wrap the contents of a html tag with that tag, instead we place the tag at the beginning, close it at the beginning, and can continue writing our transformations without having to use lambdas because we no longer are restricted to certain operators within the tag.
Code manipulation-wise, this means a lot more usage of MagicString's move.

The following is a not yet complete list of transformation ideas:

await

{#await foo}
...
{:then x}
...
{:catch e}
...
{/await}
try {
   ...
   const x = await foo;
   ...
} catch (e) {
   ...
}

each

{#each array as item, i (item.id)}
...
{:else}
...
{/each}
for (let i = 0; i < 1; i++) { // 1 is just a hardcoded placeholder
  const item = __svelte_ts_getEntry(array);item.id;
  ...
}
...

slot - let:x

<Comp {foo} let:x>
  ..
</Comp>
const comp = new Comp({ target: __sveltets_any, props: {foo: foo} });
{
  const x = comp.$$slots_def.x;
  ...
} // <- we can use these blocks to create a sub-scope which allows us to shadow variables

@jasonlyu123 thoughts?

@jasonlyu123
Copy link
Member

This might be a good idea. Needs quite some effort though. It might have some downside we can think of now. I think we can do some proof of concept. We can also check some old issues and add some type-check tests for them so we didn't break them.

About transformation ideas here is some more transformation idea:

  1. turn interpolation into a template literal. So we can still let typescript check if it's a valid expression.
  2. Maybe we can move the component end tag to somewhere valid so we can keep the language features on the end tag.

@TheComputerM
Copy link

More people use JavaScript than typescript, so can there be jsdoc definitions for events and slots with this new method? This is important to library authors to provide better DX.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Nov 9, 2021
Goal: Get rid of a tsx-style transformation and simplify transformations along the way
sveltejs#1077
dummdidumm added a commit that referenced this issue Jan 27, 2022
Goal: Get rid of a tsx-style transformation and simplify transformations along the way
#1077
#1256
#1149

Advantages:

-   no messing with projects who use JSX for some other reason and then have conflicting JSX definitions
-   TS control flow easier to keep flowing
-   overall easier transformations of Svelte specific syntax like await, each etc.
-   better type inference in some cases, for example around svelte:component

This includes:

-   rewriting the html2jsx part of svelte2tsx
-   adjusting the language server to the new transformation
-   adding a toggle to language-server and vs code extension to turn on the new transformation. Default "off" for now, with the plan to switch it to "on" and then removing the old transformation altogether
-   ensuring tests run with both new and old transformation
-   adjusting the TypeScript plugin. It uses the new transformation already now since it's not easily possible to add an option (on/off). Should be low-impact since TS doesn't need to know much about the contents of a Svelte file, only its public API

Look at the PR of this commit to see more explanations with some of the entry points of the new transformation.
@dummdidumm
Copy link
Member Author

Closing as we settled on a new transformation, opening a feedback thread before making it the new default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants