Contributing to Svelte - Fixing issue #4392

Investigating the bug

Once I verified that the behavior described in the issue is unexpected and reproducible in the latest version of Svelte, I copied the REPL code into my local machine to investigate.

I have a test-svelte folder ready in my local machine, where I created using Svelte Template. I have npm linked my local Svelte clone to the test-svelte folder, so I can rebuild test-svelte anytime with the latest changes done to my Svelte clone.

- /Projects
  - svelte                <-- cloned from https://github.com/sveltejs/svelte
  - test-svelte           <-- initialised with Svelte Template
    - node_modules/svelte <-- symlink to `/Projects/svelte`

I have yarn dev running in the Svelte folder, so any changes I make gets compiled immediately.

I prefer to build test-svelte and serve it with http-server rather than start a dev server test-svelte in watch mode. That allows me to

• Run the http-server in the background while tweaking the Svelte code or the test-svelte app.
• Not having to restart the dev server whenever I've made changes to the Svelte code
• Able to inspect and modify bundle.js without worrying that accidentaly save in the test-svelte app will overwrite the bundle.js

Looking at the different bundle.js generated from with {...spread} attributes and without spread attributes

<script>
  let value = ['Hello', 'World'];
</script>

<!-- with spread -->
<select multiple {value} {...{}}>
  <option>Hello</option>
  <option>World</option>
</select>

<!-- without spread -->
<select multiple {value}>
  <option>Hello</option>
  <option>World</option>
</select>

I found the following diffs in the bundled output:

+ let select_levels = [{ multiple: true }, { value: /*value*/ ctx[0] }, {}];
+	let select_data = {};
+	for (let i = 0; i < select_levels.length; i += 1) {
+	  select_data = assign(select_data, select_levels[i]);
+	}
- let select_value_value;

  return {
    c() {
      /* ... */
      set_attributes(select, select_data);
    },
    m(target, anchor) {
      /* ... */
-     select_value_value = /*value*/ ctx[0];

-     for (var i = 0; i < select.options.length; i += 1) {
-       var option = select.options[i];
-       option.selected = ~select_value_value.indexOf(option.__value);
-     }
    },
-   p: noop,
+    p(ctx, [dirty]) {
+      set_attributes(select, get_spread_update(select_levels, [{ multiple:   true }, dirty & /*value*/ 1 && { value: /*value*/ ctx[0] }, {}]));
+    },
    /* ... */
  };

Well, I know I haven't cover how spread attribute works in any of my "Compile Svelte in your Head" articles, but the general idea is that, Svelte builds an array of attributes, and then apply it to the element / Component.

For example, if we write the following in Svelte

<div foo="foo" {...bar} baz="baz" {...qux} />

It gets compiled to something like this:

const levels = [{ foo: 'foo' }, bar, { baz: 'baz' }, qux];
// build the attribute maps
const data = {};
for (let i = 0; i < levels.length; i++) {
  data = Object.assign(data, levels[i]);
}

// set attribute to element
for (const attributeName in data) {
  div.setAttribute(attributeName, data[attributeName]);
}

// if `bar` changed
const updates = get_spread_update(levels, [false, bar, false, false]);
// updates will return the updates needed to make, in this case, the diff in `bar`, eg: { aa: '1' }
for (const attributeName in updates) {
  div.setAttribute(attributeName, updates[attributeName]);
}

So, this roughly explains the additional code added into the bundle.js for handling spread attributes.

However the code that is removed, is something I am not familiar with.

// in `mount` method
for (var i = 0; i < select.options.length; i += 1) {
  var option = select.options[i];
  option.selected = ~select_value_value.indexOf(option.__value);
}

It seems like we are trying to set option.selected after we mount the <select> element. Not sure how important is that to us.

To verify that the bug is because that the above code snippet is missing when having a spread attribute, I tried adding the code snippet into the bundle.js manually, and refresh the page.

  // ...
  m(target, anchor) {
    insert(target, select, anchor);
    append(select, option0);
    append(select, option1);
    for (var i = 0; i < select.options.length; i += 1) {
      var option = select.options[i];
      option.selected = ~ctx[0].indexOf(option.__value);
    }
  },
  // ...

Instead of ~select_value_value.indexOf(...), I changed it to ~ctx[0].indexOf(...), as select_value_value wasn't created when using spread attribute.

...and it works!

So, now we know that the bug is caused by missing setting option.selected on mount, now its time to figure out what the code snippet is not generated when there's a spread attribute.

To quickly find out why something is not generated, I tried to look for where it is generated, figuring out probably whether certain condition was not set correctly to cause the Svelte compiler to omit out the code snippet.

To find the right place to start looking is an art. Usually I try to global search a small snippet of code that is most likely static, something that has no variable name, for example:

• .indexOf(option.__value)
• .options.length;
• .selected = ~

The only search result I got when searching for .indexOf(option.__value) is in src/runtime/internal/dom.ts

export function select_options(select, value) {
  for (let i = 0; i < select.options.length; i += 1) {
    const option = select.options[i];
    option.selected = ~value.indexOf(option.__value);
  }
}

Anything within src/runtime/ are helper functions that are referenced from the output code, to reduce the output code size. Hmm... probably we should reuse the select_options helper function:

  // ...
  m(target, anchor) {
    insert(target, select, anchor);
    append(select, option0);
    append(select, option1);
-   for (var i = 0; i < select.options.length; i += 1) {
-     var option = select.options[i];
-     option.selected = ~ctx[0].indexOf(option.__value);
-   }
+   select_options(select, select_value_value);
  },
  // ...

Anyway, src/runtime/internal/dom.ts is not where I am looking for, so I tried searching .options.length

// filename: src/compiler/compile/render_dom/wrappers/Element/Attribute.ts

updater = b`
  for (var ${i} = 0; ${i} < ${element.var}.options.length; ${i} += 1) {
    var ${option} = ${element.var}.options[${i}];

    ${if_statement}
  }
`;
block.chunks.mount.push(b`
  ${last} = ${value};
  ${updater}
`);

Yes, this is most likely where it is.

Firstly, let me update the updater to use the src/runtime/ select_options helper instead:

if (is_multiple_select) {
  updater = b`@select_options(${element.var}, ${last});`;
} else {
  updater = b`@select_option(${element.var}, ${last});`;
}

block.chunks.mount.push(b`
  ${last} = ${value};
  ${updater}
`);

The b`...`, is called a tagged template, where the b is a function that takes in the template literal and return something. In this case, the b function returns an Abstract Syntaxt Tree (AST).

The b function comes from code-red, a utility to generate a JavaScript AST node. Beside b, code-red provides a few helper functions:

• b returns a block node
• x returns an expression node
• p returns a object property node

These helper functions are useful in generating code in Svelte compiler, particularly because the placeholder itself can takes in another AST node:

const node = { type: 'Identifier', name: 'foo' };
const code = b`const ${node} = 1;`; // returns an AST node for `const foo = 1;`

@ in front of @select_option is a convention in Svelte, where it will get replaced to refer to helpr functions in src/runtime/ before writing the generated AST out:

const code = b`@foo(bar)`;
// turns into

// import { foo } from 'svelte/internal';
// foo(bar);

Coming back to figure out why this piece of code is not executed when there's a spread attribute,

if (is_legacy_input_type) {
  // ...
} else if (is_select_value_attribute) {
  if (is_multiple_select) {
    updater = b`@select_options(${element.var}, ${last});`;
  } else {
    updater = b`@select_option(${element.var}, ${last});`;
  }

  block.chunks.mount.push(b`
    ${last} = ${value};
    ${updater}
  `);
} else if (is_src) {
  // ...
}

I tried adding console.log before the if statement, to figure out the value for is_legacy_input_type and is_select_value_attribute:

console.log(
  'is_legacy_input_type:',
  is_legacy_input_type,
  'is_select_value_attribute:',
  is_select_value_attribute
);
if (is_legacy_input_type) {
  // ...
}

To my surpise, there was no log. AttributeWrapper#render wasn't executed.

I tried removing the spread attribute, and verified from the log that the AttributeWrapper#render method was indeed executed when there's no spread attribute.

To figure out the caller of the AttributeWrapper#render method, I added console.trace at the top of the method:

render(block: Block) {
  console.trace('trace');
  // ...
}

Trace: trace
  <!-- highlight-next-line -->
  at AttributeWrapper.render (/Projects/svelte/compiler.js:8269:11)
  at /Projects/svelte/compiler.js:10749:14
  at Array.forEach (<anonymous>)
  <!-- highlight-next-line -->
  at ElementWrapper.add_attributes (/Projects/svelte/compiler.js:10748:19)
  at ElementWrapper.render (/Projects/svelte/compiler.js:10472:8)
  at /Projects/svelte/compiler.js:10454:11
  at Array.forEach (<anonymous>)
  at ElementWrapper.render (/Projects/svelte/compiler.js:10453:24)
  at FragmentWrapper.render (/Projects/svelte/compiler.js:13030:18)
  at new Renderer (/Projects/svelte/compiler.js:13112:17)

This brought me to src/compiler/compile/render_dom/wrappers/Element/index.ts

add_attributes(block: Block) {
  // Get all the class dependencies first
  this.attributes.forEach((attribute) => {
    if (attribute.node.name === 'class') {
      const dependencies = attribute.node.get_dependencies();
      this.class_dependencies.push(...dependencies);
    }
  });

  if (this.node.attributes.some(attr => attr.is_spread)) {
    this.add_spread_attributes(block);
    return;
  }

  this.attributes.forEach((attribute) => {
    attribute.render(block);
  });
}

If there's a spread attribute, it will call the this.node.attributes.some(attr => attr.is_spread) method instead of calling attribute.render(block), so that's probably why AttributeWrapper#render wasn't called.

I looked into the method add_spread_attributes, found out it contain only the code about handling spread attributes as I explained earlier. It didn't have any code related to select_options, so I figured that, maybe <select multiple> with spread attribute is an edge case that wasn't handled currently at all.

So, I tried to add a special check for this case at the bottom of the add_spread_attributes method:

add_spread_attributes(block: Block) {
  // ...
  // for `<select>` element only
  if (this.node.name === 'select') {
    block.chunks.mount.push(
      b`@select_options(${this.var}, ${data}.value);`
    );
  }
}

As mentioned in the The Svelte Compiler Handbook, a block is where it keeps the code to generate the create_fragment function. The return object of the create_fragment function contains various method as mentioned in Compile Svelte in your Head, such as c(), m() and d(). To add code into different method, you can push them into the array in block.chunks, for example:

// push `const foo = 1` to `m()`
block.chunks.mount.push(b`const foo = 1`);

// push `const bar = 2` to `c()`
block.chunks.create.push(b`const bar = 2`);

I tried adding @select_options(...) into the m() method and yup, the <select> element is pre-selected correctly!