Svelte generates code referencing undefined variables like div_nodes

[pzmarzly]

Hi. I'm trying to use ESLint on the final IIFE .js that Svelte generates, in order to get the no-undef lint (usage of undefined variable).

.eslintrc.js

```js module.exports = { parserOptions: { ecmaVersion: 2019, sourceType: 'module', }, env: { es6: true, browser: true, node: true, }, plugins: [ 'svelte3', '@typescript-eslint/eslint-plugin' ], overrides: [ { files: ['**/*.svelte'], processor: 'svelte3/svelte3', extends: "eslint:recommended", }, { files: ['**/*.js'], rules: { "no-undef": "warn", }, }, ], } ```

This gives me:

/path/to/bundle.js
1668:43  warning  'form_nodes' is not defined  no-undef

Offending code in bundle.js:

            l: function claim(nodes) {
                if (default_slot) { default_slot.l(form_nodes); }
                throw new Error("options.hydrate only works if the component was compiled with the `hydratable: true` option");
            },

Form.svelte is just <form><slot /></form> (I simplified it for the purpose of this report). If I add content to the default slot, or other HTML tags (children) to <form>, the issue is still there. Searching for form_nodes and _nodes in bundle.js gives me no results aside of the function above. If I change Form.svelte content to <div><slot /></form>, I get warning about undefined div_nodes.

Are <...>_nodes in the code above intended to be undefined? How can I set them? Or what else can I do to get the results I want (getting "... is not defined" errors at compile time, not runtime)?

[Conduitry]

I think this is a harmless bug, as the l (claim) method should never be getting called without hydration. (If you compile with dev mode on, you can see that calling that method would result in throwing a 'was-not-compiled-for-hydration' exception anyway. And if you compile with hydration mode on, the _nodes variable is actually declared.)

The proper solution here seems to me to be to not have a claim method at all if we're not compiling with hydratable: true (and to have it only throw the exception if we're compiling with dev: true). It looks like this bug has existed since version 3.0.0.

[Conduitry]

Current shot at this:

diff --git a/src/compiler/compile/render_dom/wrappers/Slot.ts b/src/compiler/compile/render_dom/wrappers/Slot.ts
index 1ab1b6ab..fb90afab 100644
--- a/src/compiler/compile/render_dom/wrappers/Slot.ts
+++ b/src/compiler/compile/render_dom/wrappers/Slot.ts
@@ -137,12 +137,12 @@ export default class SlotWrapper extends Wrapper {
        block.render_listeners(`_${slot.name}`);
        block.event_listeners = listeners;

-       if (block.chunks.create) create.push(b`if (!${slot}) { ${block.chunks.create} }`);
-       if (block.chunks.claim) claim.push(b`if (!${slot}) { ${block.chunks.claim} }`);
-       if (block.chunks.hydrate) hydrate.push(b`if (!${slot}) { ${block.chunks.hydrate} }`);
-       if (block.chunks.mount) mount.push(b`if (!${slot}) { ${block.chunks.mount} }`);
-       if (block.chunks.update) update.push(b`if (!${slot}) { ${block.chunks.update} }`);
-       if (block.chunks.destroy) destroy.push(b`if (!${slot}) { ${block.chunks.destroy} }`);
+       if (block.chunks.create.length) create.push(b`if (!${slot}) { ${block.chunks.create} }`);
+       if (block.chunks.claim.length) claim.push(b`if (!${slot}) { ${block.chunks.claim} }`);
+       if (block.chunks.hydrate.length) hydrate.push(b`if (!${slot}) { ${block.chunks.hydrate} }`);
+       if (block.chunks.mount.length) mount.push(b`if (!${slot}) { ${block.chunks.mount} }`);
+       if (block.chunks.update.length) update.push(b`if (!${slot}) { ${block.chunks.update} }`);
+       if (block.chunks.destroy.length) destroy.push(b`if (!${slot}) { ${block.chunks.destroy} }`);

        block.chunks.create = create;
        block.chunks.claim = claim;
@@ -155,9 +155,11 @@ export default class SlotWrapper extends Wrapper {
            b`if (${slot}) ${slot}.c();`
        );

-       block.chunks.claim.push(
-           b`if (${slot}) ${slot}.l(${parent_nodes});`
-       );
+       if (renderer.options.hydratable) {
+           block.chunks.claim.push(
+               b`if (${slot}) ${slot}.l(${parent_nodes});`
+           );
+       }

        block.chunks.mount.push(b`
            if (${slot}) {

Seems to address this particular issue, and doesn't break any tests.

[mrkishi]

@Conduitry Do you know when chunks.claim may legitimately need to exist without options.hydratable? re: https://github.com/sveltejs/svelte/blob/8d7d0ff7dd7b248ca6d9b6b4cac098430c6e6ffa/src/compiler/compile/render_dom/Block.ts#L275

[Conduitry]

Judging by the test failures if I remove the || this.chunks.claim.length > 0 part, that only happens when compiling in dev mode and claim contains just the throw new Error about not compiling in hydratable mode. I don't know when else it might happen.

[mrkishi]

It'd probably make sense to make it hydratable || dev then? I'd take if hydratable throughout code generation to be more optimization than correctness.

[Conduitry]

diff --git a/src/compiler/compile/render_dom/Block.ts b/src/compiler/compile/render_dom/Block.ts
index f73212f3..8ea90049 100644
--- a/src/compiler/compile/render_dom/Block.ts
+++ b/src/compiler/compile/render_dom/Block.ts
@@ -272,7 +272,7 @@ export default class Block {
            }`;
        }

-       if (this.renderer.options.hydratable || this.chunks.claim.length > 0) {
+       if (this.renderer.options.hydratable || this.renderer.options.dev) {
            if (this.chunks.claim.length === 0 && this.chunks.hydrate.length === 0) {
                properties.claim = noop;
            } else {

still causes three js sample tests to fail (producing unexpected claim methods in blocks), and I'm not sure why. (Maybe something about claim being empty vs hydrate being empty?) You can keep poking at this if you'd like, but I don't think I'm going to.