sveltejs / rfcs

RFCs for changes to Svelte
274 stars 81 forks source link

RFC for adding built-in actions #24

Open kevmodrome opened 4 years ago

kevmodrome commented 4 years ago

This RFC proposes that we add built-in actions, much like built-in transitions.

As rendered.

pngwn commented 4 years ago

Interesting proposal. There is actually precedent for this both in the built-in transitions and animations and in the fact that sveltejs/gestures exists.

This is a low cost proposal that has zero impact on the compiler and only adds a moderate maintenance burden. If we could figure out a sensible set of built-ins, then I'd be happy with this. Of course an action library could be provided as an additional package (under the Svelte org) but I see no reason why actions should be treated differently to other directives. transitions actually used to be in a separate library and were moved to the Svelte package itself as of version 3.

swyxio commented 4 years ago

anyone have objections/additions to these builtins?

the only one im not sure about is lazyload... what would that look like?

kevmodrome commented 4 years ago

They were just examples at first, I'm not sure which ones are actually actions one would use often. The lazyLoad one I was thinking about was something like this: https://svelte.dev/repl/f12988de576b4bf9b541a2a59eb838f6?version=3.23.2

Edit: the "real" action would probably need some options exposed to customise it accordingly.

swyxio commented 4 years ago

ah cool. yeah might want to add in a "fade-in" option for that stuff like this thing from @babycourageous https://svelte.dev/repl/9a6087b0b661445ead0b45c0c15d273a?version=3.23.2

how about establishing criteria for what is built-in worthy and what is not?

babycourageous commented 4 years ago

use:selectTextOnFocus is a generic piece of functionality that might be a good one for the mix (is this another classic from @kevmodrome ?): https://svelte.dev/repl/fe73b0e2f5824804a7f80c5379652294?version=3.21.0

kevmodrome commented 4 years ago

Yes, it would be great if some core peeps could weigh in on what they would need to OK any actions. I think the selectTextOnFocus one is really nice. Another one could be validating inputs, something like this: https://svelte.dev/repl/201c420c3406424e87388a948efaa5eb?version=3.23.2 @antony was this your REPL?

I think we could try to figure out what different kinds of developers would need. I know personally that someone working at an agency oftentimes has very different requirments than someone working at a product company (is that even a term in english?)

@babycourageous Not this time, although it is a great one! It's from @christianheine https://medium.com/better-programming/practical-svelte-the-use-directive-60635671335f :)

christianheine commented 4 years ago

Hey there, did I see someone mention my name? :)

@kevmodrome: I think this RFC is a great idea. I just went though my current codebase and checked what I use most. It's actually mostly related to inputs (e.g. clearTextOnEscape, selectTextOnFocus, blurOnEscape, etc.).

And for things related to focus (e.g. to issue a custom event when the user clicks outside an element).

Here are my personal favorites (already in Typescript):

/** Selects the text inside a text node when the node is focused */
export function selectTextOnFocus(node: HTMLInputElement) {

  if (!node || !node.tagName || node.tagName.toLowerCase() != 'input') {
    throw Error('Action can only be used with <input> tags');
  }

  const handleFocus = (_event: FocusEvent) => {
    node && typeof node.select === "function" && node.select();
  };

  node.addEventListener("focus", handleFocus);

  return {
    destroy() {
      node.removeEventListener("focus", handleFocus);
    }
  };
}

/** Clears the text inside a node when Escape is pressed */
export function clearTextOnEscape(node: HTMLInputElement) {

  if (!node || !node.tagName || node.tagName.toLowerCase() != 'input') {
    throw Error('Action can only be used with <input> tags');
  }

  const handleKey = (event: KeyboardEvent) => {
    if (event.key === "Escape" && node && node.value) {
      event.preventDefault();

      node.value = "";

      const inputEvent = new Event("input", {
        bubbles: true,
        cancelable: true,
      });

      node.dispatchEvent(inputEvent);
    }
  };

  node.addEventListener("keydown", handleKey);

  return {
    destroy() {
      node.removeEventListener("keydown", handleKey);
    }
  };
}

/** Blurs the node when Escape is pressed */
export function blurOnEscape(node: HTMLElement) {

  const handleKey = (event: KeyboardEvent) => {
    if (event.key === "Escape" && node && typeof node.blur === "function" && !event.defaultPrevented) node.blur();
  };

  node.addEventListener("keydown", handleKey);

  return {
    destroy() {
      node.removeEventListener("keydown", handleKey);
    }
  };
}

/** Blurs the node when Enter is pressed */
export function blurOnEnter(node: HTMLElement) {

  const handleKey = (event: KeyboardEvent) => {
    if (event.key === "Enter" && node && typeof node.blur === "function" && !event.defaultPrevented) node.blur();
  };

  node.addEventListener("keydown", handleKey);

  return {
    destroy() {
      node.removeEventListener("keydown", handleKey);
    }
  };
}

Close events. I use the latter for all sorts of Dropdowns, Modals, etc..


/** Dispatch close event on the press of Escape */
export function closeOnEscape(node: HTMLElement) {

  const handleKey = (event: KeyboardEvent) => {
    if (event.key === "Escape" && node) {
      node.dispatchEvent(
        new CustomEvent("close")
      );
    }
  };

  document.body.addEventListener("keydown", handleKey);

  return {
    destroy() {
      document.body.removeEventListener("keydown", handleKey);
    }
  };
}

/** Dispatch close event on scroll */
export function closeOnScroll(node: HTMLElement) {

  const handleScroll = (_event: Event) => {
    node.dispatchEvent(
      new CustomEvent("close")
    );
  };

  document.addEventListener("scroll", handleScroll);

  return {
    destroy() {
      document.removeEventListener("scroll", handleScroll);
    }
  };
}

/** Dispatch close event on click outside of node */
export function closeOnClickOutside(node: HTMLElement) {

  const handleClick = (event: MouseEvent) => {
    if (node && event.target && !event.defaultPrevented) {

      node.dispatchEvent(
        new CustomEvent("close")
      );
    }
  };

  document.addEventListener("click", handleClick, true);

  return {
    destroy() {
      document.removeEventListener("click", handleClick, true);
    }
  };
}

/** Dispatch close event on click outside of node */
export function closeOnFocusOutside(node: HTMLElement) {

  const handleFocusIn = (event: FocusEvent) => {
    if (node && event.target && !node.contains(event.target as HTMLElement) && !event.defaultPrevented) {
      node.dispatchEvent(
        new CustomEvent("close")
      );
    }
  };

  document.addEventListener("focusin", handleFocusIn, true);

  return {
    destroy() {
      document.removeEventListener("focusin", handleFocusIn);
    }
  };
}

I also wrote drag & drop as actions, but I never managed to get them to be truly reusable (i.e. without making some assumptions about the contents). Let me know if that is something you are interested in.

kevmodrome commented 4 years ago

Nice! A lot of good ones in there.

kevmodrome commented 4 years ago

Dropping this progressive form enhancement action that Rich posted on twitter: https://svelte.dev/repl/167e7c7a05844e3dab686b4257641d73?version=3.23.2

babycourageous commented 4 years ago

GENIUS

swyxio commented 4 years ago

so @christianheine's actions are an interesting test of the proposed acceptance criteria for built-in actions. i believe it is actually more lines of code than just using event binding:

<script>
/** Selects the text inside a text node when the node is focused */
function focus(event) {
  event.target.select();
}
</script>
<input type="text" on:focus />

because it isn't an action, it doesnt have to do all those defensive checks against its own misuse, and it is extremely clear to the user how to modify it in future. if we exported import {selectTextOnFocus} from 'svelte/actions' it would either cease to be useful the moment the user needs to do something slightly different, or need a bunch of hard-to-maintain and hard-to-remember options.

same deal for attaching listeners to document.body:

<script>
/** Execute business logic on the press of Escape */
function keydown(event) {
    if (event.key === "Escape") {
      /* business logic here */
    }
}
</script>
<svelte:body on:keydown/>

how would you disagree with this? just want to make sure that the benefit from an extra abstraction outweighs the cost of an abstraction.

babycourageous commented 4 years ago

@sw-yx I had the same thoughts when writing up some scroll actions - abstract out into action or just use svelte:window to attach listener. Good question indeed.

christianheine commented 4 years ago

@sw-yx / @babycourageous : I fully see your arguments and also got similar feedback regarding actions before.

My view on this: The more I used Svelte (and I guess this applies to most technologies), the more I realized that there's always a plethora of options to accomplish almost anything.

So my personal focus shifted to choose the options that*:

*unless they sacrifice performance when it matters

<input use:clearTextOnEscape ... not only helps to remove duplicated function declarations in each component (thereby reducing component size & maintenance effort), but it also clearly indicates what this element does. Without having to create component containers all the time (especially when paired with some well-chosen common global CSS declarations, which by the way can live perfectly well along Svelte in-component styles)

Over the last 6 months, my startup built a fairly large app using Svelte. At times, certain components had more than 1000 lines of code, which made them a real pain to reason about. Actions (along with well-chosen component composition & Typescript for non-Svelte code) made a big difference to get the codebase to behave again.

Below is how a simple modal looks like now. While this is an almost simplistic example, it's also what I love about the way of coding with Svelte. Without having to play code golf, you can create really compact, readable code. It also proves your point: The on:click handler is inline because the modal was the only place where this particular feature was ever needed ;-).

<script>
  import { createEventDispatcher } from "svelte";
  import { fade } from "svelte/transition";
  import { closeOnEscape } from "../library/svelte";

  const dispatch = createEventDispatcher();

  export let duration = 200;
  export let dark = false;
</script>

<div class="ui-modal"
     class:dark
     transition:fade={{duration}}
     use:closeOnEscape
     on:click|self={() => dispatch("close")}
     on:close>
  <div class="ui-modal__content">
    <slot/>
  </div>
</div>
babycourageous commented 4 years ago

@christianheine RIGHT! I can't believe I flaked on the duplicated logic that actions helps solve. That makes total sense.

And 100% - Svelte definitely presents many ways to approach the same problem. I like your two checks for how to approach the choice.

swyxio commented 4 years ago

cool, I can see that, though personally I would still make closeOnEscape a reusable function rather than an action. well, I don't intend to be a decision maker here :) just want to help discuss criteria and make svelte users' lives better. I don't know what's next in the RFC process.

slimboyfats commented 4 years ago

Would actions like enter viewport and leaves viewport be usable in this context or are they to general?

babycourageous commented 4 years ago

Hm, if it's a mouse entering/leaving viewport. I would guess one-off using svelte:window. If it's a specific element, probably action-worthy

Could also use bind directive on the component's element if it's one-off which speaks to the more than one way to solve things that Svelte offers us.

I'm personally finding myself reaching for actions more and more over bind but that's admittedly not based on any logical finding and more to try new ways to solve old problems (also due to @kevmodrome's talks and Discord answers, hehe)

slimboyfats commented 4 years ago

I'm thinking more of the lazyload version above but also for a "leave / out of viewport".

Something like this:

<Comp use:viewportAction on:enterViewport={doStuffIn}  on:leaveViewport={doStuffOut} /> 
Wolfr commented 4 years ago

I really like this proposal, because it can get the action functionality to a wider audience. I think it's a powerful and underused part of Svelte.

The current action tutorial is a bit complicated.

We could do with a simpler example like closing something when clicking outside, or using actions to create a tooltip (example REPL).

I find that the current examples, the pannable and longpress, are things you probably won't need in regular web development.

swyxio commented 4 years ago

@slimboyfats not to bikeshed too much on api design but what about <Comp use:viewport={{enter: doStuffIn, leave: doStuffOut}} />. the main pitch for this is so we dont have too much API clutter since these are official actions.

anyway - what next? can we agree on a small set to get this ball rolling? do we agree on acceptance criteria?

slimboyfats commented 4 years ago

@sw-yx Looks good ☺️

My question was more towards if this kind of actions where a good idea as it would make an easy and unified way for detecting enter / leave the viewport.

kevmodrome commented 4 years ago

@sw-yx I think we really need the core maintainers to weigh in on this. There's no point in building something until then, imo :/

antony commented 4 years ago

I can't speak for the rest of the maintainers, but I really like this idea.

However, what I can say is there is absolutely no harm in building a nice library of decent, general-purpose actions, with a view to publishing them independently (say, svelte-actions), and then them becoming a part of the Svelte Org umbrella, as has happened to a number of other projects:

In fact I would suggest this as the number 1 route to them becoming part of the official organisation, since time and resource are scarce, and we have an awful lot of things that we could probably weigh-in on, but it's hard to prioritise them as mere ideas.

I'd say this collection of actions would be an absolute winning project anyway (certainly I would be a user), so why not create it, get it in a good state, and then prove it's worth. Worth a thousand words!

pngwn commented 4 years ago

One thing I'd like to point out is some thing can be actions but probably shouldn't be. As has been pointed out, actions are incredibly powerful. The main reason for this is that Svelte is a DOM framework and actions allow you direct access to the DOM. You can essentially do everything in an action.

The issue is that you are limited in what Svelte APIs are available to you, and also in what APIs you expose. Anything that might need the flexibility of adding custom markup or styling, for example, probably makes more sense as a component. Actions also cannot be composed in the same way that stores and components can.

lazyLoad is an interesting cases, this could work well as an action because you can do what you want with image primitive, you can wrap it however you want and add as much styling to the containers or image element itself which is great. On the other hand, if you want to hook into the lazy loading lifecycle, the best you can do is expose some callback options, there are no other API options available to you.

I'm also not convinced a lazyLoad action will actually work with an SSRed or Static app: on receiving the SSRed html, the browser will start to render and fetch those images , then the javascript will run but it is too late. The alternative is to not use a src attribute which is user hostile as it would fail without JavaScript and not something I'd be keen to recommend. Ideally you would offer some way to opt out of lazy loading for SSR but this isn't easily doable via an action because they don't run at all in SSR mode, a component would make customising this behaviour much easier, since they do run in SSR. I guess the takeaway here, is if you want control over what happens in SSR and DOM modes then an action doesn't give you much flexibility, it is just on or off.

Custom events are a natural choice for actions, so while the tutorial example is a little complex it is one of the perfect use-cases: it is a complex custom event (or set of events), it exposes an API that is very similar to the on:* API, it only concerns the node itself and nothing else. There used to be a custom event API in v2 and it was removed because actions could do everything custom events could, this is definitely one of their intended use-cases.

To be generally useful, I think actions should generally as generic as possible, I imagine functions that return actions are often more useful in some cases. closeOnEscape for example feels like it needs to know quite a lot about its context, onEscape is fine but then would you need one for every key? An onKey(key, callback) action generator seems the best of both.

This is one of my favourite actions: autoresizing textarea

@sw-yx Regarding the SSR comment, actions interact with nodes and don't run in SSR (there are no nodes). They are an ideal candidate for progressive enhancement (Rich's PE-form is an extreme but clever example of this) but should never be relied on for functionality outside of an SPA. See my note about lazy loading.

Edit: Added REPL link to action.

milkbump commented 4 years ago

A note on having a Svelte-sponsored lazyLoad action. There a handful of caveats to lazy loading regarding when to load:

type of image...whether Lite mode is enabled on Chrome etc, ...effective connection type

and viewport size etc.

Native lazy loading handles the device, network, and browser edge cases for you with zero JS and is more SSR friendly. There's decent support —about 70% as of this writing and soon to be 80% once it's no longer behind a flag in Safari macOS and iOS.

I suggest letting the browser handle this one for us.

swyxio commented 3 years ago

alright gang i have made a start on a prototype library as suggested: https://github.com/sw-yx/svelte-actions we can bikeshed individual actions here and add them.

npm i svelte-actions

see readme for what i've included: https://github.com/sw-yx/svelte-actions#included-actions

happy to donate this library to the sveltejs org if we get enough support

swyxio commented 3 years ago

just released v0.1 with the 6 actions we like so far:

perhaps worth keeping it minimal. https://github.com/sw-yx/svelte-actions/releases/tag/v0.1.0

didier commented 2 years ago

Is this still being worked on?

swyxio commented 2 years ago

thanks to @gtm-nayan for helping me with the types of the svelte-actions repo - still being maintained! try it out and give feedback! https://github.com/sw-yx/svelte-actions/releases/tag/v0.2.2

CarlosIvanchuk commented 2 years ago

I heard about usehooks-ts on the purrrfect.dev podcast and I thought that some of the hooks might serve as inspiration to add new actions (or utility functions) to this package. Additionally, mantine has a set of hooks that are divided into 4 categories: State Management, UI and DOM, Utilities and Lifecycle. Perhaps hooks on UI and DOM are also ideal candidates?