Values API default values

[leastbad]

We know that you're working hard on getting v2 ready for release, so it seems like a good time to formally propose an enhancement to the specification proposed in #202 to support providing default values.

The best syntax I've seen so far was proposed by @dancallaghan in his comment on the PR:

static values = {
  url: String,
  refreshInterval: [Number, 300],
  loadOnConnect: [Boolean, true]
}

I love this because it's non-intrusive and falls back to the current syntax.

[paulozoom]

In that example, [Number, 300] feels redundant because 300 is already a Number. So Stimulus would also need to deal with inconsistency between declared type and default type (or decide not to, and error).

Couldn’t it be instead either the type or the value?

static values = {
  url: String,
  refreshInterval: 300,
  loadOnConnect: true,
}

The type could always be inferred from the value. (Even if typeof Array being Object is a little annoying, the Object case would always be further checked for the Array type.)

[leastbad]

Sure, I could be happy with either. It's true that [String, 5] or [Number, 'twelve'] aren't helpful.

Thanks for the suggestion! I hope @sstephenson and @javan agree.

[leastbad]

A little bird told me that @dhh is accepting PRs on Stimulus, now. To prepare, here is my first attempt at writing TypeScript in a first draft implementation of @paulozoom's syntax suggestion.

https://github.com/leastbad/stimulus/tree/default_values

https://github.com/leastbad/stimulus/commit/af84ded12c8b78fed543317298db129332b9ae45

I know that it's not as pretty as the code around it, but I'm very open to and excited about suggestions for making the code better. Again, I don't know TypeScript so a lot of what I did involved disabling strict typing.

Here's a test controller:

import { Controller } from 'stimulus/dist/stimulus.umd'

export default class extends Controller {
  static values = {
    url: 'bill',
    interval: 5,
    params: { name: 'sue' }
  }

  connect () {
    console.log(this.urlValue, this.intervalValue, this.paramsValue)
  }
}

[dhh]

I dig this default values design where types are inferred from the values. So 👍 from here.

[leastbad]

What I want to know is what you were doing for the rest of the 5 minutes @dhh
(5 minutes is an incredible response time)

[javan]

Can you share any real world examples where you'd this? I'm not opposed to the change, but also can't find any good use cases in our own controllers.

[swanson]

Can you share any real world examples where you'd this? I'm not opposed to the change, but also can't find any good use cases in our own controllers.

I could see myself using this in places where I'm wrapping other libraries in Stimulus controllers. Often times they are configuration options that I want to expose in the markup, but I don't think it's a huge pain right now.

Example: wrapping popper.js and allowing the placement to be overridden (but defaulting if none provided)

import { Controller } from "stimulus";
import Popper from "popper.js";

export default class extends Controller {
  static targets = ["menu", "toggle"];
  static values = { placement: String }

  toggle({ currentTarget }) {
    this.popper = new Popper(this.toggleTarget, this.menuTarget, {
      placement: this.placementValue || "top"
    });
  }

  ...
}

[leastbad]

I'm just as surprised that you folks don't need default values! 😀

Over the year that I've been working with the values API, my work in Stimulus has been split pretty evenly between the StimulusReflex client library (a giant Stimulus controller), controllers I create for my own applications, and controllers that I package up and distribute via the NPM registry. The category that demands default values is the distributed packages. These play the same role jQuery plugins used to, and can have potentially dozens of configuration options.

A very grokable example would be stimulus-image-grid, which defines three parameters as Stimulus Values. These all have intelligent defaults in the public-facing API, and without a default Values API, you have to define them manually:

    if (!this.hasPaddingValue) this.paddingValue = 10
    if (!this.hasTargetHeightValue) this.targetHeightValue = 150
    if (!this.hasDisplayValue) this.displayValue = 'inline-block'

Does this make sense?

[olimart]

Same here, this pattern keeps repeating in shared controllers.

get placement() {
  return this.placementValue || "left"
}

[swanson]

I'm just as surprised that you folks don't need default values! 😀

It's not that I don't need defaults, just that I don't find coalescing them to be particularly annoying with ||. I'd happily switch because it is nicer to have the defaults in the values struct directly, just not a burning need for me personally :)

[adrienpoly]

Default value would make it easier to publish npm packages for reusable controllers having options.

The value API is perfect to create those options. Default values would make it simpler rather than having new getters as @olimart pointed. Having to write those getters kind of break the initial intention of the value API to remove getters/setters.

get placement() {
  return this.placementValue || "left"
}

[dancallaghan]

Can you share any real world examples where you'd this? I'm not opposed to the change, but also can't find any good use cases in our own controllers.

Just had a look through some of HEY's controllers and saw some instances where this hypothetically could be implemented. The aim here is not to say HEY's code is bad, just using it as an example.

// contollers/autoclick_controller.js
export default class extends ApplicationController {
  static values = { delay: Number, clicked: Boolean }

  connect() {
    this.timeout = setTimeout(this.clickOnce, this.delayValue)
  }

You may very well want the delayValue to default to 0, but there are situations where this is not the case.

The shorthand || operator also has some issues. This example would still default to 400:

connect() {
  this.timeout = setTimeout(this.clickOnce, this.delayValue || 400)
}

<button data-controller="autoclick" data-autoclick-delay-value="0">Witness me</button>

This would force using the aforementioned ternary approach:

connect() {
  this.timeout = setTimeout(this.clickOnce, this.delay)
}

get delay() {
  return this.hasDelayValue ? this.delayValue : 400;
}

This doesn't look too bad, but you lose the context of the Value in the setTimeout. You could put the ternary inline, but again I don't find it particularly nice and it's not reusable.

Previous example with the suggested changes. If you want to know the default value it's right at the top of the controller, not in some getter function.

export default class extends ApplicationController {
  static values = { delay: 400, clicked: Boolean }

  connect() {
    this.timeout = setTimeout(this.clickOnce, this.delayValue)
  }

<button data-controller="autoclick">I'll wait</button>
<button data-controller="autoclick" data-autoclick-delay-value="0">
  Witness me
</button>

Sorry about the contrived example, but I don't think it's too far a stretch.

[p8]

Another example would be renaming a boolean value to the opposite if you prefer.

For example in Hey's autocomplete_controller.js forbidAdding:

export default class extends ApplicationController {
  static values = { url: String, forbidAdding: Boolean }
  ...

Could be renamed to allowAdding:

export default class extends ApplicationController {
  static values = { url: String, allowAdding: true }
  ...

[dhh]

Continued discussion in #350.