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

" " (space) in text nodes replaced with "" (null string) in each blocks (svelte@5.0.0-next.97+) #11932

Closed
adam-coster opened this issue Jun 6, 2024 · 13 comments · Fixed by #11944
Milestone

Comments

@adam-coster
Copy link

adam-coster commented Jun 6, 2024

Describe the bug

If you have a component that creates a bunch of text nodes via an {#each} loop, on HMR the spaces between those nodes disappear (see video). On page refresh the spaces come back, so this appears to be an HMR problem.

svelte-hmr-issue.mp4

I tested Svelte versions and found that this issue started in svelte@5.0.0-next.97. Summary of tested versions (✅ means it worked as expected, ❌ means the issue was observed):

- ❌ 5.0.0-next.150
- ❌ 5.0.0-next.100
- ❌ 5.0.0-next.97
- ✅ 5.0.0-next.96
- ✅ 5.0.0-next.95
- ✅ 5.0.0-next.93
- ✅ 5.0.0-next.87
- ✅ 5.0.0-next.75
- ✅ 5.0.0-next.50
- ✅ 4.2.17 

It appears that on HMR the text nodes are being replaced such that:

💡 Any text node whose data was " " gets replaced with a nullstring ""

Reproduction

You can observe this behavior in the Svelte 5 REPL, but there you can only see it with the spaces removed. For a full reproduction, do the following:

  • Create a new SvelteKit application (e.g. npm create svelte@latest hmr-whitespace)
  • Replace the package.json contents with the following to ensure the same dependency versions:
    {
      "name": "hmr-whitespace",
      "version": "0.0.1",
      "private": true,
      "scripts": {
        "dev": "vite dev",
        "build": "vite build",
        "preview": "vite preview"
      },
      "devDependencies": {
        "@sveltejs/adapter-auto": "^3.2.1",
        "@sveltejs/kit": "^2.5.10",
        "@sveltejs/vite-plugin-svelte": "^3.1.1",
        "svelte": "5.0.0-next.97",
        "vite": "^5.2.12"
      },
      "type": "module"
    }
  • Install deps (npm install)
  • Replace the contents of hmr-spaces/src/routes/+page.svelte with:
    <script>
      let string = "Where did the spaces go?";
      const words = string.split(/( )/); // Note that regex groups in `.split` *keep* the spaces, unlike when there is no group
      console.log(words);
    </script>
    
    <p>
      {#each words as word}
        {word}
      {/each}
    </p>
  • Run and view the page (npm run dev)
  • Observe that the content looks as expected (the text has spaces)
    • In the Chrome console, run [$('p')] to inspect the element. Observe that its childNodes include Text nodes whose data value is a space (" ")
  • Edit the value of the string variable to trigger HMR
  • Observe that the text has lost its spaces
    • In the Chrome console, run [$('p')] to inspect the element. Observe that the Text nodes in its childNodes that used to be spaces (" ") are now null strings ("").
  • Refresh the page
  • Observe that the spaces have returned

The video above shows what you should observe.

Repeating this test with any Svelte version above 5.0.0-next.97 yields the same outcome. All versions less than 5.0.0-next.97 do NOT yield this outcome and work as expected.

Logs

In the Chrome console, the only log is:


[vite] hot updated: /src/routes/+page.svelte

Similarly, in the Node console:

9:28:45 AM [vite] hmr update /src/routes/+page.svelte

System Info

See the package.json content listed above for project dependency versions.

Additional system info:

    OS: Windows 11 10.0.22631
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700KF
    Memory: 34.77 GB / 63.82 GB
  Binaries:
    Node: 20.11.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.2.0 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Edge: Chromium (125.0.2535.67)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    svelte: 5.0.0-next.97 => 5.0.0-next.97

Severity

This behavior completely breaks my application

@dummdidumm dummdidumm added this to the 5.0 milestone Jun 6, 2024
@adam-coster adam-coster changed the title HMR removes spaces between text nodes in svelte@5.0.0-next.97+ HMR replaces " " (space) in text nodes with "" (null string) in svelte@5.0.0-next.97+ Jun 6, 2024
@paoloricciuti
Copy link
Member

I don't think it's HMR i think is simply hydration...the reproduction works in the REPL too!

@adam-coster
Copy link
Author

adam-coster commented Jun 6, 2024

Ah, good to know! I originally couldn't reproduce it in the REPL, but didn't try again once I figured out what was happening.

@Conduitry
Copy link
Member

@dummdidumm This is the intended behavior, isn't it? Isn't this just the new {#each} behavior in v5? https://svelte-5-preview.vercel.app/docs/breaking-changes#whitespace-handling-changed

@adam-coster
Copy link
Author

This case doesn't seem to fall into the listed cases there:

  • "Whitespace between nodes is collapsed to one whitespace"
    • These whitespace text nodes are not being collapsed into a single space, they are being set to null strings.
  • "Whitespace at the start and end of a tag is removed completely"
    • These are whitespace text nodes, not at the beginning or start of a tag
      Certain exceptions apply such as keeping whitespace inside pre tags

And the behavior on full reload versus dynamic reload differ, which presumably is not intended in any case.

@paoloricciuti
Copy link
Member

Btw i don't even think this is hydration...is probably the each loop thing with a bug

REPL

@adam-coster
Copy link
Author

It does look like <svelte:options preserveWhitespace={true} /> solves the problem, so it's no longer blocking my use case.

However, I did find this behavior to be surprising and, more importantly, very difficult to pin down. Searching for related terms in GitHub Issues and more generally didn't yield any answers. Maybe that'll be improved once the Svelte 5 docs are more discoverable, though the docs don't actually indicate that this behavior should happen.

@adam-coster
Copy link
Author

Crap, I take it back. <svelte:options preserveWhitespace={true} /> does NOT solve the problem for my case. If I've split the original string on any non-word character, I end up with new spaces inserted between every text node: REPL

For clarity, my use case is that I have a "spellcheck" component that splits a string up into labeled parts (words, template variables, other). It then loops through those parts and either renders them directly with {substring} or wraps them in some HTML. It looks something like this:

{#each parsed as part (part.start)}
	{#if part.isVariable}
		<code>{part.substr}</code>
	{:else if !part.isWord && part.substr === '\n'}
		<br />
	{:else if !part.isWord || part.valid}
		{part.substr}
	{:else if part.isWord}
		<span
			role="button"
			class="spelling-error"
			onclick={async (event) => {
				if (event.ctrlKey) await glossary.addTerm(part.substr);
			}}
		>
			{part.substr}
		</span>
	{/if}
{/each}

@Conduitry
Copy link
Member

When preserveWhitespace mode is on, then the whitespace between the {#each words as word} and the {word} and the {/each} is now significant, so I'd say that behavior is expected.

If, after you've split the string, you've lost the information about which elements should have whitespace displayed between them, then there's not anything Svelte is going to be able to do. If you want to conditionally display whitespace, and you know what that condition is, then you could do something like {string + (condition ? ' ' : ''} or something like {string}{#if condition}{' '}{/if}.

@adam-coster
Copy link
Author

adam-coster commented Jun 6, 2024

The information is not lost. I'm still rendering the whitespace characters via {theWhitespaceChar}. They just become "" text nodes once rendered instead of " " text nodes.

You can see in this REPL that the split text includes entries that are just whitespace strings, and these are not being excluded by the loop. You can also see each entry in the text nodes of the dom, but that their data is set to "" instead of " ".

@Conduitry
Copy link
Member

Gotcha, okay, yeah, it looks like the core of the bug, then is something like: {#each ['foo', ' ', 'bar'] as str}{str}{/each}

Apologies, I had missed that the original REPL had .split(/( )/) rather than .split(' ') / .split(/ /). That's actually a new bit of JS regex behavior to me.

@adam-coster
Copy link
Author

No worries! .split(/( )/) definitely has unexpected behavior if you haven't used it before 🙂

@adam-coster
Copy link
Author

And yep, your minimal case is exactly right: REPL

@adam-coster adam-coster changed the title HMR replaces " " (space) in text nodes with "" (null string) in svelte@5.0.0-next.97+ each-loops replace " " (space) in text nodes with "" (null string) when looping in svelte@5.0.0-next.97+ Jun 6, 2024
@adam-coster adam-coster changed the title each-loops replace " " (space) in text nodes with "" (null string) when looping in svelte@5.0.0-next.97+ " " (space) in text nodes replaced with "" (null string) in each blocks (svelte@5.0.0-next.97+) Jun 6, 2024
@paoloricciuti
Copy link
Member

I should have a fix for this ...PRing now

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