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

Implement {#with} block construct #4601

Closed
wants to merge 4 commits into from

Conversation

RedHatter
Copy link
Contributor

@RedHatter RedHatter commented Mar 25, 2020

This PR implements the {#with} block constructs to be used as the following.

{#with someExpression as someVariable}
  <p>{someVariable}</p>
{/with}

This construct would be useful to alias deep values, destructure values, or to avoid recalculating expensive operations. There are times such aliasing can be done using a computed variable but this is not always the case and can make the template less readable.

There are at least two situations where it is not possible to use a computed variable.

  1. When attempting to alias a value from inside of an each block.
  2. When trying to alias a value set using a let directive from inside of a slot.

While it is true that this can be accomplished in the above cases using a child component, this isn't always desirable, such as when only a few lines use the alias or when decoupling from the rest of the template would be nontrivial.

Alternative syntax

  • {#alias a as b}
  • {#let b = a}
  • {#set b = a}
  • {#assign b = a}
  • {#assign a to b}

modifier: (node: Node) => Node;
}

function unpack_destructuring(contexts: Context[], node: Node, modifier: (node: Node) => Node) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would obviously be simplified once #4596 is merged.

@Conduitry
Copy link
Member

So I can find it when I'm making the changelog, this would close #1521.

@Conduitry
Copy link
Member

I also notice that in #4169, I expressed doubts about the name with because of its existing meaning in javascript, which is a fair point from my past self.

@arxpoetica
Copy link
Member

arxpoetica commented Mar 30, 2020

What if we supported some kind of destructuring?

{#each items as { var1, var2 }}
  <p>{var1} and {var2}</p>
{/each}

@RedHatter
Copy link
Contributor Author

RedHatter commented Mar 30, 2020

@arxpoetica Deconstructing is already supported by both each blocks and my pull request.

@Conduitry

I expressed doubts about the name with because of its existing meaning in javascript, which is a fair point from my past self.

Well with statements are deprecated and I don't know if enough developers even know they exist for it to be an issue. Then again there's no reason not to change the block name.

I'm open the renaming it if you think it's a good idea. What alternative name should we use? I posted a few at the end of my original post but I'm not really partial to anything.

@dimfeld
Copy link
Contributor

dimfeld commented Apr 8, 2020

Personally, I like with for this type of thing. But if that is voted down my second preference would be for let, probably #let expr as name for consistency with #each.

@Oreilles
Copy link

Oreilles commented Apr 14, 2020

I personally much prefer the with syntax. I'd love to see this feature added.

@PaulMaly
Copy link
Contributor

PaulMaly commented Apr 14, 2020

Workaround for some cases:

{#await obj then { foo, bar }}
  {foo}
{/await}

@knobo
Copy link

knobo commented Apr 15, 2020

I vote for calling it let Could be used like this {#let foo = initialValue}. It's closer to javascript, and it does not make problems down the road. If one wants to invent things that is not related to javascript. Maybe you would want to use {#with} for something, like temporary components or server rendering functionality, debugging, or whatever... Maybe javascript will invent with, then it would be best to not confuse the developer with a let/with mix.

Until then, abuse the language like this:

{#await {} then {foo = 10, bar = 20}}
   {foo}/{bar}
{/await}

Edit: I forgot.. Javascript actually has the with construction. Which you should not use..

If svelte should have with It should be used like this:

{#with  {foo: "bar"}}
  {foo}
{/with}

Which would display the string "bar"

@pushkine
Copy link
Contributor

either way it doesn't make sense in the xml tree
I think that this is a problem that cannot be fixed using existing svelte syntax

@RedHatter
Copy link
Contributor Author

Rebased off master and added changes from #4596.

@Conduitry Any thoughts based on the discussion? If you think we need more user feedback how do we go about that?

@pngwn
Copy link
Member

pngwn commented May 17, 2020

I don't think anyone has raised this issue yet but I think this is a bad idea from a design perspective. One of Svelte's advantages, for me, is that the template is kept relatively clear of logic due to the constraints of the syntax. This forces developers to put the bulk of their logic in the script where you would expect it to be and also keeps it in one place. This feature muddies that line and paves the way for arbitrary logic anywhere in the template.

I don't understand the value of being able to alias variables directly in the template and the other uses cases can be dealt with by preparing your data ahead of time, instead of performing computations directly in an each block.

Additionally, I think this kind of API addition should go through the RFC process, it changes the template constraints pretty significantly and should be discussed in full.

@knobo
Copy link

knobo commented May 17, 2020

One of Svelte's advantages, for me, is that I can test out ideas with relatively few lines of code. the #with feature could save me from adding a separate component for the content of an #each loop.
I get frustrated when I have to create a new file, move the content of the #each clause, import it as a component, and add attributes and create exports for that, and implement events to send messages back, and event handlers, when I just wanted to test a small feature.

When I'm prototyping components I like to manage the data where it appears, and not send it back and forth if there is no reason for it. I also don't like to be forced by a language to do things a certain way.

If #with is a problem, I'd rather write some guidelines on limiting its usage.

In some cases, I could also create a component without any <script> tag at all. So in that way, I could actually bulk up the logic in one place if I could get some help from the #with block.

@pngwn
Copy link
Member

pngwn commented May 17, 2020

the #with feature could save me from adding a separate component for the content of an #each loop.

How? You can still do everything inside a loop if you need to, you certainly don't need to split it out to a separate component. I'd love to see an example of the problem that this actually solves.

I also don't like to be forced by a language to do things a certain way.

Svelte forces you do to do all kinds of things in a very specific way (as does every other framework/library), those constraints generally make for a better experience. Adjustments are always needed but that is the trade-off. This makes tools, pattern and integrations simpler. It makes Svelte code across projects more consistent. This is basically a design goal at this point, even though it has evolved organically.

If #with is a problem, I'd rather write some guidelines on limiting its usage.

Guidelines do very little. Developer discipline is not a reliable method of encouraging best practices, they have to be enforced to be reliable. If something isn't a great idea, the tools should support that approach. Adding an API for something is an endorsement of that approach.

@BulatDashiev
Copy link

BulatDashiev commented May 18, 2020

I'd love to see an example of the problem that this actually solves.

For me with #with i can run helper function once in template

{#await fetch(...) then data}
  {#with prepare(data) as value }
     <h1>{value}</h1>
     ...
     <p>{value}</p>
  {/with}
{/await}

Instead of calling prepare twice to get value from data. Of course this can be done by doing fetch().then(prepare), but there might be some complex examples where #with can be useful.

@knobo
Copy link

knobo commented May 18, 2020

Something like this, maybe....

{#each array as element}
  {#with status}
     {#await status}
         Updating...
     {:then value}
         <button on:click={ () => status = fetch(element.url) }>{value}</button>
     {/await}
  {/with}
{/each}

@knobo
Copy link

knobo commented May 19, 2020

Or something like this:

<script>
  let files;
  function uploader(node, file) {
    const reader = new FileReader();
    const ref = firebase.storage().ref();
    const fileRef = ref.child(file.name);
    const task = fileRef.put(file);

    task.on('state_changed', function progress(snapshot) {
      const percentage = (snapshot.bytesTransferred / snapshot.totalBytes) * 100;
      let event = new CustomEvent("progress", { detail:  percentage });
      node.dispatchEvent(event);
    });

    reader.addEventListener("load", function () {
      node.src = reader.result;
    }, false);

      reader.readAsDataURL(file);
  }
</script>

<input type="file" multiple bind:files>
{#if files}
  <ul>
  {#each files as file, index}
    {#with progress = 0}
    <li>Upload: 
        <progress id="file" bind:value={progress} max="100"> {progress}% </progress>
        <img on:progress={ ({detail}) => progress = detail }
             use:uploader={ file } style="height: 50px;" />
    </li>
    {/with}
  {/each}
  </ul>
{/if}

@knobo
Copy link

knobo commented May 19, 2020

Or, better written like this. Though, if stores could be defined in a {#with} statement is another subject.

<script>
  import { tweened } from 'svelte/motion';

  let files;

  const upload = (file) => {
    // simulate upload
    const progress = tweened(0, {delay: 500});
    progress.set(100);
    return progress;
  } 
</script>

<input type="file" multiple bind:files>
{#if files}
  {#each files as file}
    {#with progress = upload(file)}
      <progress id="file" value={ $progress } max="100" />
    {/with}
  {/each}
{/if}

@knobo
Copy link

knobo commented May 19, 2020

Without {#with} and without creating a new component, it could be done like this:

<script>
    import {tweened} from 'svelte/motion';

    let files;
    let progress = [];
    const upload = (file, index) => {
        // simulate upload
        const p = tweened(0, {delay: 1000});
        p.set(100);
        p.subscribe(val => {
            progress[index] = val
        });
    }

    $: files && Array.from(files).forEach((file, i) => upload(file, i));
</script>

<input type="file" multiple bind:files>

{#if files}
    {#each files as file, index}
        <progress value={ progress[index] || 0 } max="100" />
    {/each}
{/if}

<style>
    progress {
        display: block;
    }
</style>

@Rich-Harris
Copy link
Member

I've just opened an RFC that proposes a similar addition, {@const x = ...}. This comment addresses the main difference between the proposals.

@RedHatter RedHatter closed this Jun 26, 2021
@Conduitry Conduitry mentioned this pull request Nov 21, 2021
@ethanlal04
Copy link

Whenever I encountered this issue, this is what I did:

{#each [someExpression] as someVariable}
  <p>{someVariable}</p>
{/each}

It works and its easy to understand what it does. I don't see why we need a #with block when we can already do this. Is there some kind of performance issue with this or does the #with just get rid of the brackets?

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

Successfully merging this pull request may close these issues.

None yet