chore: remove binding.expression

[Rich-Harris]

hal-fixing-lightbulb.gif

This started out with #12511. As mentioned in that PR, I'd like to move more of the each block optimisation to compile time, if possible. My first step was to get rid of $.unwrap(i) in cases like this...

{#each things as thing, i}
  <p>{i}: {thing.name}</p>
{/each}

$.each(node, 1, () => things, $.index, ($$anchor, thing, i) => {
  var p = root_1();
  var text = $.child(p);

  $.reset(p);
-  $.template_effect(() => $.set_text(text, `${$.unwrap(i) ?? ""}: ${$.unwrap(thing).name ?? ""}`));
+  $.template_effect(() => $.set_text(text, `${i}: ${$.unwrap(thing).name ?? ""}`));
  $.append($$anchor, p);
});

...because it's a bit daft to call $.unwrap when we know perfectly well that i is a) not a signal, and b) defined, making the ?? "" pointless. (Also, unwrap relies on brittle duck typing and should be removed.)

But it wasn't that simple, because in some contexts the index might be wrapped in a source, and in others (i.e. the key function) not. Which made me happy because there's a part of our codebase that I've wanted to get rid of for a long time, and I finally have a good reason: binding.expression.

Essentially, we assign an expression to certain bindings when we have specific transformation requirements. But it's not really in keeping with how the rest of the compiler works — ideally, transformation would be pure, and all the mutatey stuff would happen during the analysis phase. (Imagine a world in which it's possible to generate server and client output from a single compiler run, rather than the current — wasteful — process of parsing and analysing from scratch before handing the result off to the transformation phase. While that's not a near term goal, reducing the number of places where mutation can happen in order to make code more understandable and robust is always a goal.)

And we have a perfectly good mechanism already — context.state. This PR adds a getters record to TransformState that allows nodes to create those ad hoc overrides without mutating the bindings themselves. This should make it much easier to fix the i thing above, and no doubt other things too.

Right now it only uses getters in one place, hence draft status.

• needs to have an equivalent setters record to replace binding.mutation will do this in a follow-up

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: b445acd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

Marked this ready for review. Right now it probably looks like a step backwards from a certain angle, but this is a first step towards making all this stuff a lot more flexible and robust — I anticipate that the state.getters[name] approach can replace serialize_get_binding in its entirety. More excitingly, I think we can do the same thing for binding.mutation and get rid of serialize_set_binding, which is a horrifically complicated function that has proven somewhat resistant to refactoring.

[trueadm]

LGTM