Svelte: Private methods

[kylecordes]

Currently, when a component defines a method, that method becomes part of the public API the component. But it appears that many methods are not really intended to be part of the public API at all; instead, they exist only so that they can be called by an event handler on something inside of a component. For example, consider svelte-virtual-list (which I had nothing to do with other than picking this particular snippet at random to use as an example:)

https://github.com/sveltejs/svelte-virtual-list/blob/master/src/VirtualList.html

It defines two methods, but as far as I can tell, neither of them is intended to be part of the public API.

Therefore, a feature request: add a way to create private methods.

Ideas for how to mark a private method:

• Create a different key (parallel to methods:) where private methods are put.
• Use a naming convention; for example perhaps methods that start with _ would be private.
• When TypeScript support arrives, perhaps automatically interpret a language level private method as a component level private method.

[Rich-Harris]

I like the idea of using a leading underscore — it's an established convention, and it would be fairly easy to do. It'd be a breaking change though, so I guess we'd need to have a privateMethods: true compiler option or similar until v3 if we were to do that.

[lukeed]

Would the compiled version hoist the function above the component definition?

[Rich-Harris]

That's my thinking, yeah. And then for this...

<button on:click="_increment('count')">
  clicks: {count}
</button>

<script>
  export default {
    methods: {
      _increment(key) {
        const { [key]: val } = this.get();
        this.set({ [key]: val + 1 });
      }
    }
  };
</script>

...we'd either do this...

function _increment(key) {
  const { [key]: val } = this.get();
  this.set({ [key]: val + 1 });
}

// later...
div.addEventListener('click', () => {
  _increment.call(this, ctx.key);
});

...or this (note there's no call, I believe this might have miniscule perf advantages? Also it can minify better):

function _increment(component, key) {
  const { [key]: val } = component.get();
  component.set({ [key]: val + 1 });
}

// later...
div.addEventListener('click', () => {
  _increment(component, ctx.key);
});

[lukeed]

Ya sounds good. Another option is:

let __incr = _increment.bind(this);
div.addEventListener('click', __incr);

I think it will come down to how we apply the svelte helpers for event modifications and whatnot

[Rich-Harris]

One wrinkle. Imagine you have something like this (from an image gallery app I'm working on):

methods: {
  _handle_keydown(event) {
    if (event.which === 37 || event.which === 39 || event.which === 32) {
      // space or right arrow goes forwards, left arrow goes backwards
      const d = event.which === 37 ? -1 : 1;
      this._navigate(d);
    }
  },

  _handle_click(event) {
    // clicking on most of the page goes forwards, clicking on the left third goes backwards
    const d = event.clientX < window.innerWidth / 3 ? -1 : 1;
    this._navigate(d);
  },

  _navigate(d) {
    const { count, params } = this.get();

    const id = clamp(+params.id + d, 1, count);
    goto(`images/${id}`);
  }
}

If underscore-prefixed methods weren't actually on the instance, this._navigate(...) wouldn't work.

[lukeed]

It should, binding is the same as your first option with call.

Anyway you slice this, it sounds good!

[Rich-Harris]

this.navigate(...) would work, but this._navigate(...) wouldn't — it's not the this that's the problem, it's the _navigate

[Conduitry]

We're already rewriting this to component in some other places (hoisted event handlers I think?) - does it sound practical to rewrite this._navigate to _navigate.bind(this) or _navigate.call(this, ...)?

edit: Or is that too much magic?

[lukeed]

Oh derp. I just woke up :)

What about defining var placeholders (above the component) and call them directly?

var _navigate, _handle_click, _handle_keydown;

//... inside oncreate

_handle_keydown = function (event) {
  if (event.which === 37 || event.which === 39 || event.which === 32) {
      // space or right arrow goes forwards, left arrow goes backwards
      const d = event.which === 37 ? -1 : 1;
      _navigate(d); // EASY TRANSFORMATION
    }
  }.bind(this); // OPTIONAL, compiler can infer if needed

  _handle_click = function (event) {
    // clicking on most of the page goes forwards, clicking on the left third goes backwards
    const d = event.clientX < window.innerWidth / 3 ? -1 : 1;
    _navigate(d);
  }.bind(this);

  _navigate = function (d) {
    const { count, params } = this.get();

    const id = clamp(+params.id + d, 1, count);
    goto(`images/${id}`);
  }.bind(this);

And then passing thru Uglifyjs or whatever will still keep that all consistent

[Rich-Harris]

It's a lot of magic. It would work in 95% of cases, but you can wind up in some pretty rocky territory:

this[d === 1 ? '_increment' : '_decrement']();

More broadly, any time you passed the instance to a function that the compiler couldn't 'see' (e.g. you have a helper function that operates on several components with the same private methods), things fall apart.

[lukeed]

Can write private methods into an object with matching key names and similar bindings (where needed). Then any this._ would be obj._... still magic but may bump that number up to 98%

[Rich-Harris]

But then this.set(...) inside one of those methods would fail... unless we wanted to get really fancy and start throwing Proxies around

[lukeed]

This is kinda what I meant:

// Original.html
<h1>Hello {name}!</h1>

<button on:click="_onClick('Click')">Single Click</button>
<button on:dblclick="_onClick('Double')">Double Click</button>

<script>
    export default {
        methods: {
            _onClick(str) {
                this._setName(str);
            },
            _setName(name) {
                this.set({ name });
            }
        }
    }
</script>

// Transform.html
<h1>Hello {name}!</h1>

<button ref:a1>Single Click</button>
<button ref:a2>Double Click</button>

<script>
    let privada = {};
    privada._onClick = function (str) {
        privada._setName(str);
    };

    export default {        
        oncreate() {
            // bind _setName here bcuz relies on component API
            privada._setName = function (name) {
                this.set({ name });
            }.bind(this);

            // attach listeners to references
            let r = this.refs;
            r.a1.onclick = function (ev) {
                privada._onClick('Click');
            };
            r.a2.ondblclick = function (ev) {
                privada._onClick('Double');
            };
        }
    }
</script>

I mean, it definitely requires a lot of compiler insight to make this happen, but I don't think it requires much more than passing around component arguments either.

I think that because Svelte's component API (and properties) are so few, it wouldn't be too gross to check for them explicitly when parsing the private method(s). If and when a this.set, this.get, or this.refs (etc.) is used, move the privada.* definition into the oncreate block, so that you can grab a context reference. Otherwise, everything else can be be defined above/outside of the component.

In this way, the case you pointed out:

this[d === 1 ? '_increment' : '_decrement']();

...wouldn't matter since all the private methods were defined in the correct place with the correct bindings.

[PaulMaly]

@Rich-Harris @lukeed @kylecordes
Great issue! Described much of my out thoughts!

I think the main problem, not in method's privacy:

<script>
   import privateMethod from '...';

   export default {
        oncreate() {
              privateMethod(); // use as private
        }
   };
</script>

But in missing separation between methods (public api) and event handlers. So, most of my event handlers shouldn't be used outside of event listeners. But they can be used because it's become a part of public api.

I believe that all the things were already implemented in Ractive )))

<script>
   import privateMethod from '...';

   export default {
        oncreate() {
              privateMethod(); // use as private
        },
        methods: {
             publicMethod() { ... }
        },
        on: {
             click() { ... }
        }
   };
</script>

So, I vote for obvious separation between methods and event handlers rather than hacks with underscored methods or something.

[Rich-Harris]

This is another issue that is solved by RFC 1, so I'll close this