sveltejs / language-tools

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

Unexpected token. Did you mean `{'>'}` or `>`? #143

Closed mgholam closed 4 years ago

mgholam commented 4 years ago

svelte

Describe the bug On a perfectly good app.svelte file I'm getting the above.

System (please complete the following information):

dummdidumm commented 4 years ago

Could you paste the complete code of that file?

jasonlyu123 commented 4 years ago

This is probably because of the code before it got transformed with a missing brackets. Can you provide the code before it?

mgholam commented 4 years ago
<script>
  import { onMount } from "svelte";
  import u from "./Helper/utils.js";
  import msgbox from "./Components/msgbox.js";
  import Tabs from "./Components/tabs.svelte";
  import Button from "./Components/Button.svelte";
  import InboxTab from "./Main/Inbox.svelte";
  import ArchiveTab from "./Main/Archive.svelte";
  import HelpTab from "./Main/Help.svelte";
  import SettingsTab from "./Main/Settings.svelte";
  import DocOutTab from "./Main/DocOut.svelte";

  let mainid = "mainid";
  let server = "";
  let TABS;
  let timer = null;
  // let RTL = false;

  onMount(() => {
    u.addPrototypes();

    TABS.addtab({
      name: "inbox",
      description: "$Inbox$",
      icon: "gi-save-file", //icons.faFolderOpen
    });
    TABS.addtab({
      name: "docout",
      description: "$Docs Out$",
      icon: "gi-open-file", //icons.faFilter
    });
    TABS.addtab({
      name: "archive",
      description: "$Search Letters$",
      icon: "gi-search", //icons.faFilter
    });
    TABS.addtab({
      name: "settings",
      description: "$Settings$",
      icon: "gi-cog", // icons.faCog
    });
    TABS.addtab({
      name: "help",
      description: "$Help$",
      icon: "gi-question-sign",
    });

    setTimeout(() => {
      // kludge to get the server for fetch
      u.GET("/api/getuser").then((r) => (server = r));

      // server = window.ServerURL;
      u.GET("/api/inbox.getsettings").then((data) => {
        if (data.Color) {
          u.SetTheme(data.Color);
        }
        if (data.Language == "fa") {
          document.body.dir = "rtl";
          // document.documentElement.style.setProperty("--tblAlign", "right");
        }
      });
      TABS.setInitial();
    }, 0);

    // Heartbeat for auto update
    timer = setInterval(() => {
      u.GET("/api/getuser").catch(() => {
        clearInterval(timer);
        msgbox.Ok("$Connection Failed!$", "", () => window.location.reload(true));
      });
    }, 10 * 1000);
  });

  // register all tabs to be created here for Tab component
  function createTab(tabname, id, props) {
    var tab = null;

    switch (tabname) {
      case "inbox":
        tab = new InboxTab({
          target: document.getElementById(id),
        });
        break;
      case "help":
        tab = new HelpTab({
          target: document.getElementById(id),
        });
        break;
      case "archive":
        tab = new ArchiveTab({
          target: document.getElementById(id),
        });
        break;
      case "settings":
        tab = new SettingsTab({
          target: document.getElementById(id),
        });
        break;
      case "docout":
        tab = new DocOutTab({
          target: document.getElementById(id),
        });
        break;
    }

    return tab;
  }

  // function configeditor() {
  //   // console.log("here");
  // }
</script>
dummdidumm commented 4 years ago

Pasting the given code into an App.svelte of a starter template works for me. Anything else we should know about your setup or the file?

mgholam commented 4 years ago

I'm not an expert, so you will have to tell me what/where to look for.

jasonlyu123 commented 4 years ago

Can you also provide the markup and the skyle tag?

mgholam commented 4 years ago

I found the culprit:

<svelte:head>
  <link rel="stylesheet" href="/lib/jodit.es2018.min.css" />
  <script src="/lib/jodit.es2018.min.js"> 

  </script>
</svelte:head>

It's the script tag in the head section that messes up the parser.

dummdidumm commented 4 years ago

This is another case where our regex-parser fails. Maybe we should switch to using a html parser instead like svelte2tsx does already - but we need to check if that handles the case correctly or also fails. There we can check if the script/styles have a parent, and if not, ignore it. Maybe we can share the logic and extract it as a util from svelte2tsx which svelte-language-server uses.

mgholam commented 4 years ago

I've opened another request regarding the optimization of the vs code extension size on disk (currently 150mb). Can you comment on this? [I have deleted the typescript and prettier folders from the svelte-language-server\node_modules and all seems well, but you could "build" everything and not rely on the node_modules right?]

dummdidumm commented 4 years ago

I saw this and did not have time to look into this yet. It's weekend and we all do this in our free time. Also please keep issues separate which don't have anything to do with each other.

mgholam commented 4 years ago

Thanks.

swyxio commented 4 years ago

@dummdidumm interested in picking up this first issue. is the idea that we should use htmlxparser.ts instead of the regex parsing? are there any other alternatives you haven't discussed?

dummdidumm commented 4 years ago

Yes, the idea would be to make the findVerbatimElements (here) function available to the language server by also exporting it through the index.ts, and then adjust it so that both svelte2tsx and svelte-language-server can use it. For svelte-language-server it would replace the inner workings of the extractTag (here) function. I'm not 100% sure though they can share it completely, if it differs too much we might as well duplicate some of the stuff and not do the "export from svelte2tsx" stuff. So for example right now findVerbatimElements does find all script/style tags in the htmlx, but for svelte-language-server we only want the top level ones. There are tests on both sides for this, so you should get feedback quickly if it's working or not.

An alternative would be to somehow enhance the regex to only look for top level scripts/styles, but I feel this gets too complicated for a regex then.

swyxio commented 4 years ago

ok i'm going to err on the side of duplicating it. clean code is overrated :)

swyxio commented 4 years ago

i have set up a test for this inside of https://github.com/sveltejs/language-tools/blob/master/packages/language-server/test/lib/documents/utils.test.ts but I am not sure it is the right way to test it:

it('ignores script tag in svelte:head', () => {
            // https://github.com/sveltejs/language-tools/issues/143#issuecomment-636422045
            const text = `
            <svelte:head>
                <link rel="stylesheet" href="/lib/jodit.es2018.min.css" />
                <script src="/lib/jodit.es2018.min.js"> 
                </script>
            </svelte:head>
            <p>jo</p>
            <script>top level script</script>
            <h1>Hello, world!</h1>
            <style>.bla {}</style>
            `
            assert.deepStrictEqual(extractTag(text, 'script'), {
                // ????
            });
        })

it doesnt give the Unexpected token error but it does match the wrong <script> tag here. is there a better way to test?

swyxio commented 4 years ago

for whoever sees this in future - i ended up completely not duplicating it at all - the requirements for svelte language server (top level tags only) are very different from svelte2tsx (all tags)

mgholam commented 4 years ago

The problem still exists in 99.0.39

dummdidumm commented 4 years ago

Turns out it's not only a problem with extractTag. It's also a bug in svelte2tsx, output has a syntax error:

// ....
<sveltehead>
  <link rel="stylesheet" href="/lib/jodit.es2018.min.css" />
  <script src=""/lib/jodit.es2018.min.js"">  // <<<-- two `"` each, syntax error

  </script>
</sveltehead>
mgholam commented 4 years ago

The problem still exists in 99.0.41

dummdidumm commented 4 years ago

I cannot reproduce the bug with the latest version, given your code snippet with <svelte:head>...

mgholam commented 4 years ago

sv

dummdidumm commented 4 years ago

Could you post a minimum snippet that reproduces the error for you?

mgholam commented 4 years ago
<script>
  import { onMount } from "svelte";
  let aa = false;
  onMount(() => {  // <--- the problem seems to be here at =>
    aa = true;
  });
</script>

<svelte:head>
  <link rel="stylesheet" href="/lib/jodit.es2018.min.css" />
  <script src="/lib/jodit.es2018.min.js">

  </script>
</svelte:head>
dummdidumm commented 4 years ago

Can reproduce with this, thanks.

dummdidumm commented 4 years ago

The problem is that svelte2tsx will set any script it comes across as the primary script instance, so last one wins (relevant function). Nesting is not taken into account.

My first attempt at fixing that was to take into account the parent node of the walk, but that does not work because parent is not always the expect parent.

To fix that I tried to extract the logic @sw-yx has implemented into svelte2tsx so a common ground can be used by both svelte2tsx and language-server. This works, but I then stumbled across another problem: parse5 is very html spec compliant, also when it comes to self-closing tags. That means that something like <SelfClosing /> <p>bla</p> will not parse as we would expect - the node would also span <p>bla</p> and more, until parse5 would see a real closing tag (</SelfClosing>).

In short: We have to find another way than parse5 to extract our top level scripts.

Short Test that fails with the current implementation:


        it('can extract with self-closing component before it', () => {
            const extracted = extractTag('<SelfClosing /><style></style>', 'style');
            assert.equal(!!extracted, true);
        });
mgholam commented 4 years ago

Tried this as a workaround but it fails also:

<script>
  import { onMount } from "svelte";
  let aa = false;
  onMount(mount);

  function mount() {
    aa = true;   // <---error here
  }
</script>

<svelte:head>
  <script src="/lib/jodit.es2018.min.js">

  </script>
</svelte:head>
dummdidumm commented 4 years ago

As a workaround you need your top level script after the script in svelte:head. Last script wins and is treated as the top level script.

<svelte:head>
  <script src="/lib/jodit.es2018.min.js">
  </script>
</svelte:head>
<script>
  import { onMount } from "svelte";
  ....
</script>

(yes, that would mean you have to change your format setting or disable formatting completely while this is not fixed)

mgholam commented 4 years ago

Can you do what the svelte compilers does (just asking)?

dummdidumm commented 4 years ago

This error has not much to do with the svelte compiler. It's an error which occurs while doing a transformation from svelte code to jsx code using svelte2tsx.

swyxio commented 4 years ago

@dummdidumm nice work/investigations. sounds like we should use https://babeljs.io/docs/en/babel-parser instead?

dummdidumm commented 4 years ago

Will that parse a html string into a correct AST? Isn't that for js(x)/ts(x) only? Basically we need some html parser like parse5 only that it also deals with self-closing tags (too bad there's not a parse option for that in parse5...). It should also be sufficiently fast.

Or someone finds a regex that works and we revert to using that, but it is unlikely we find something without massive workarounds.

swyxio commented 4 years ago

we dont need it to be exactly correct 🤷

also i want to go back to what @mgholam said. why cant we use svelte's own parse function? guarantees absolute parity esp if we only care about top level tags

dummdidumm commented 4 years ago

We can do that as soon as https://github.com/sveltejs/svelte/issues/4818 is dealt with. Maybe use acorn/acorn-loose in the meantime?

swyxio commented 4 years ago

i dont know enough to make a call here. i'm not confident that using the svelte parse function is the best choice for this usecase. does the svelte parser do everything we need for language-server and svelte2tsx? if so, then i would rather work on https://github.com/sveltejs/svelte/issues/4818 than temporarily add acorn-loose only to drop it later on.

dummdidumm commented 4 years ago

svelte2tsx already relies heavily on the svelte parser, and in language-server we need it in SveltePlugin, and apart from that we only need HTML parsing to get the script/style tags. So relying on the parser would be a good choice I think.

mgholam commented 4 years ago

As a workaround you need your top level script after the script in svelte:head. Last script wins and is treated as the top level script.

Can you make the first script "win" and be the top level script? (as a thought...)

dummdidumm commented 4 years ago

Yes this could be a good quickfix in the meantime. I also have another idea, will have to test it out.

dummdidumm commented 4 years ago

Third time's a charm - let's hope it's fixed with the next deployment in about 12 hours.

I created a separate issue for the self-closing-tag-bug (#194)

mgholam commented 4 years ago

Woo Hoo! Thanks!

mgholam commented 4 years ago

Regression if the script block is commented :

<svelte:head>
    <!-- <script src="/lib/jodit.es2018.min.js">
  </script>-->
</svelte:head>
dummdidumm commented 4 years ago

Cannot reproduce with your snippet.

mgholam commented 4 years ago
<script type="ts">
  import { onMount } from "svelte";
  // let aa = false;
  // let pp: string = "";
  // onMount(mount);

  // function mount() {
  //   aa = true;
  // }

  // function click() {
  //   pp = "ppppp";
  //   console.log("hello");
  // }
</script>

<svelte:head>
  <!-- <script src="/lib/jodit.es2018.min.js">

  </script> -->
</svelte:head>

<div>
  <button on:click={click}>hello</button>
  <input bind:value={pp} />

</div>
dummdidumm commented 4 years ago

Fix available soon.

mgholam commented 4 years ago

Still exists in 101.9.3

<script >
  import { onMount } from "svelte";

  // } // <- error here

</script>

<svelte:head>
  <!-- <script src="/lib/jodit.es2018.min.js">

  </script> -->
</svelte:head>
azhar1598 commented 3 years ago

I have encountered this issue while i was developing my code in VScode editor.And,there is nothing wrong with the code and there are no problems showing in the terminal and Program is also executing very well.And then i just restarted my vscode editor ,everything got resolved I mean the error which was quoted in the question. Just try restarting your editor.Maybe,it might get resolved.

jasonlyu123 commented 3 years ago

Probably not related. The codebase has changed a lot since then. If you have a > character as text in the template there is a PR right now. Else you can provide a reproducible in a new issue.

unclebay143 commented 2 years ago

For anyone facing this issue: I uninstalled prettier, restart VScode, and re-install prettier and everything works fine. This is the second time this happened and that same step resolved it.

lohitakshsingla0 commented 2 years ago

Even I am facing the same issue:

` const SortCards = ({listData}) => {

return (
  <div>
    <select>
    listData.map((result) => (<option>{result.strCategory}</option>))
  </select>
  </div>
)

}

export default SortCards; `

lohitakshsingla0 commented 2 years ago

For anyone facing this issue: I uninstalled prettier, restart VScode, and re-install prettier and everything works fine. This is the second time this happened and that same step resolved it.

I tried this, but doesn't worked for me

dummdidumm commented 2 years ago

Your code snippet is JSX code, Svelte does not support JSX.