sveltejs / language-tools

The Svelte Language Server, and official extensions which use it
MIT License
1.21k stars 196 forks source link

Bug: DocumentSnapshot.ts is not passing preprocessed markup to svelte2tsx #339

Open nickreese opened 4 years ago

nickreese commented 4 years ago

Edit:

Initially I though this was a feature request, but the more I've dug into it it seems that there is a bug causing markup preprocessing not to take place.

DocumentSnapshot.ts is passing the raw string of the svelte file instead of the preprocessed svelte file to svelte2tsx.

This results in a valid preprocessor not being supported by the VSCode editor.

This may be by design as I'd imagine VSCode may show strange errors to users who don't realize their code is being rewritten before it has the TS validator run on it.


Hi, I'm working on a full fledged SSG that is designed to be Svelte's answer to Gatsby. It is designed for SEO focused, mainly static sites that have some highly interactive components. It has easy to reason about data flow, is highly pluggable, and honestly I think it has a chance at becoming 'the framework' for people building SEO sites using a SSG.

We're already using this SSG in production at https://elderguide.com but are working to make it more user friendly before releasing it.

The key feature that has been blocking me from using Svelte 100% for templating is partial hydration.

Thanks to the help from @kev on Discord, we've come up with a pretty elegant solution, but I can't get VS code to accept the custom directive.

Example of usage:

<!-- Parent -->
<script>
  import HydrateMe from './HydrateMe.svelte';

  const lotsOfData = {
    stuff: true,
    name: 'Nick',
    email: 'yep@yep.com',
    notThisOne: [1, 2, 3, 4],
  };
</script>

<!-- problemsome syntax -->
<HydrateMe interactive:data={{ name: lotsOfData.name, email: lotsOfData.email }} />

<!-- Hydrate Me -->
<script>
  export let name;
  export let email;
</script>

<div>{name} {email}</div>

Custom Directive:

<HydrateMe interactive:data={{ name: lotsOfData.name, email: lotsOfData.email }} />

svelte.config.js:

const partialHydration = require('./partialHydration');

module.exports = {
  preprocess: partialHydration,
};

partialHydration.js

module.exports = {
  markup: async ({ content, filename }) => {
    const matches = content.matchAll(/<([a-zA-Z]+)\s+interactive:data={(.*)}/gim);

    for (const match of matches) {
      console.log('match', match);
      const componentName = match[1];
      const dataFunction = match[2];

      const replacement = `<div class="needs-hydration" data-component="${componentName}" data-data={JSON.stringify(${dataFunction})}`;
      content = content.replace(match[0], replacement);
    }

    return { code: content };
  },
};

Under the Hood

To be clear, how this is working is the SSR version of the component is built with the preprocessor. Then when we parse what is returned from the SSR component, we have a small wrapper which adds a new root component when it finds <div class="needs-hydration" ...

The data found in data-compontent is added as the props on the new root component.

Questions

1 -- Is there a way I can get VS code to accept this custom directive? Here is the error I'm seeing after restarting VS code to make sure the language server had the latest svelte.config.

image

2 -- I know the language server is limited to 1 preprocessor based on #279. What needs to be done to unblock this? Where would I start?

nickreese commented 4 years ago

So the more I'm digging into the svelte2tsx project, I'm not seeing where svelte preprocessors that handle markup are being run. This may explain why my preprocessor doesn't have an impact on how the language server is doing things.

nickreese commented 4 years ago

It appears that in this code block on htmlxparser.ts the preprocessors should be run before const svelteHtmlxAst.

export function parseHtmlx(htmlx: string): Node {
    //Svelte tries to parse style and script tags which doesn't play well with typescript, so we blank them out.
    //HTMLx spec says they should just be retained after processing as is, so this is fine
    const verbatimElements = findVerbatimElements(htmlx);
    const deconstructed = blankVerbatimContent(htmlx, verbatimElements);

    //extract the html content parsed as htmlx this excludes our script and style tags
    const svelteHtmlxAst = compiler.parse(deconstructed).html;

    //restore our script and style tags as nodes to maintain validity with HTMLx
    for (const s of verbatimElements) {
        svelteHtmlxAst.children.push(s);
        svelteHtmlxAst.start = Math.min(svelteHtmlxAst.start, s.start);
        svelteHtmlxAst.end = Math.max(svelteHtmlxAst.end, s.end);
    }
    return svelteHtmlxAst;
}
nickreese commented 4 years ago

The TSX being returned shows the preprocessor isn't being run.

<><script>
  import HydrateMe from './HydrateMe.svelte';

  const lotsOfData = {
    stuff: true,
    name: 'Nick',
    email: 'yep@yep.com',
    notThisOne: [1, 2, 3, 4],
  };
</script>

<HydrateMe hydrate:client={{ name: lotsOfData.name, email: lotsOfData.email }} />
</>
nickreese commented 4 years ago

Still tracing this:

preprocessSvelteFile() in DocumentSnapshot.ts which invokes svelte2tsx isn't getting the preprocessed markup.

jasonlyu123 commented 4 years ago

The main problem is the preprocess will transpile typescript to javascript so that we can no longer type check. It somehow need a way to only preprocess the markup.

nickreese commented 4 years ago

@jasonlyu123 Was discussing this with @halfnelson on Discord. He mentioned:

We would need to run preprocessors in the LsP before calling svelte2tsx and manually skip the typescript one Which is doable I guess since there is only one typescript preprocessor atm But might be tricky to make genric We would also have to source map lookup after preprocessing So the errors are in the right spot I have done a bunch of work on getting svelte.preprocess to generate a combined sourcemap, but that isn't merged yet

dummdidumm commented 4 years ago

We also need to consider performance, since preprocessing on every keystroke can be very slow, especially if it's done for all parts. I would start with only preprocessing markup.

dummdidumm commented 4 years ago

Another big problem: the ts language service is synchronous, so the retrieval of the snapshot has to be, too. But preprocessing can be asynchronous.

kaisermann commented 4 years ago

Far from ideal, but for the async/sync issue there's always the deasync package which can make an asynchronous function synchronous. jest-transform-svelte uses it because jest transforms are also required to be synchronous.

dummdidumm commented 4 years ago

Not sure if we can use this since it seems it does need to know your node version to properly install, but for the extension we bundle everything in advance.

dummdidumm commented 4 years ago

Maybe this could help us https://github.com/sindresorhus/make-synchronous

probablykasper commented 3 years ago

Hey, has there been any progress on this?

dummdidumm commented 3 years ago

Another library which could help us forcing synchronous code https://github.com/rx-ts/synckit

fedorovvvv commented 2 years ago

Hi, it's been a year) The problem is not solved?🥲