leongersen / noUiSlider

noUiSlider is a lightweight, ARIA-accessible JavaScript range slider with multi-touch and keyboard support. It is fully GPU animated: no reflows, so it is fast; even on older devices. It also fits wonderfully in responsive designs and has no dependencies.
https://refreshless.com/nouislider/
MIT License
5.66k stars 659 forks source link

Use a template literal type for behaviour option #1144

Closed RikuVan closed 3 years ago

RikuVan commented 3 years ago

The Typescript support could be improved a little I think for the behaviour by using a template literal type. I am not sure there is a more terse way to do this but at least the Behaviour could be a union type like this:

type Behaviour = 'tap' | 'drag'...

and then in the interface it could be a union of the potential concatenations:

interface Options {
  ...
  behaviour:  Behaviour
        | `${Behaviour}-${Behaviour}`
        | `${Behaviour}-${Behaviour}-${Behaviour}`
        | `${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}`
        | `${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}`
        | `${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}-${Behaviour}`;
]

Admittedly this is kind of ugly and with a mapped type maybe (?) there is a way to do this more elegantly. But I think it would be pretty cool to get some type completion adding this option.

leongersen commented 3 years ago

Thanks for your feedback! Admittedly, the behaviour option being a string was a mistake (as was using the British spelling). I'm planning to replace it with an object basically matching what is now the (internal) Behaviour interface, but I haven gotten around to that yet. I addition to that, and to keep this change (initially) backward compatible, I think your suggestion would be very helpful. I don't think there are more than 3 behaviours to be combined at any one time.

RikuVan commented 3 years ago

There doesn't seem to be a script to run the tests with Qunit. What is your setup (or the easiest setup) for running them locally (eg from cli, in IDE...)?

RikuVan commented 3 years ago

Nevermind I guess it is as simple as opening slider.html in a browser. Maybe I am too used to overcomplicated test setups 😅 .

RikuVan commented 3 years ago

@leongersen so is it correct there can be a variation of at most 3 of these: type Behaviour = 'drag' | 'fixed' | 'snap' | 'hover' | 'unconstrained'?

So also we would want that no behaviour can be repeated: Would this suffice:

type Behaviour = 'drag' | 'fixed' | 'snap' | 'hover' | 'unconstrained'

const a: BehaviourPermutation<'drag'> = 'drag';
const b: BehaviourPermutation<'drag-fixed'> = 'drag-fixed';
const c: BehaviourPermutation<'drag-fixed-snap'> = 'drag-fixed-snap';

const d: BehaviourPermutation<'drag-fixed-drag'> = 'drag-fixed-drag'; // Error
const e: BehaviourPermutation<'snap-hover-hover'> = 'snap-hover-hover'; // Error
const f: BehaviourPermutation<'unconstrained-unconstrained-unconstrained'> = 'unconstrained-unconstrained-unconstrained'; // Error

type BehaviourPermutation<T> =
    (T extends `${infer A}-${infer B}`
        ? (B extends `${infer BA}-${infer BB}`
            ? (
                `${A}-${Exclude<Behaviour, A>}-${Exclude<Exclude<Behaviour, BA>, A>}`
            )
            : (`${A}-${Exclude<Behaviour, A>}`)
        )
        : T
    );

example

RikuVan commented 3 years ago

While the above works; I am doubtful it is the best solution since it requires the user to apply the type, via a generic. Also it seems from the issues that now all possible combinations are actually valid. Maybe it would be best to actually list (it a little onerous, all the valid combinations).

RikuVan commented 3 years ago

I am closing as I realized that this will be a problem for consumers with older typescript versions.

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.