0

So we recently had an issue where a value was accidentally added to an array initialization which caused runtime issues.

we have an object that exports some values for Image Contrasts,

export const IMAGE_CONTRAST_VALUES = {
  min: -90,
  base: 0,
  max: 100,
};

Then we have an array that consume those values,

const CONTRAST_VALUES = [
  IMAGE_CONTRAST_VALUES.min,
  IMAGE_CONTRAST_VALUES.base,
  IMAGE_CONTRAST_VALUES.base,
  IMAGE_CONTRAST_VALUES.max,
];

The issue was that we had 2 base entries being added to the array on init. This was never caught as the type is number[], Ideally to prevent this in the future we would have a type that requires that all the keys are included in the array, as well as in the proper order, to make sure base isn't set as the first entry of the array etc..

Initially we thought that creating a tuple of

type ImageManipulationTuple = [number, number, number]

would solve this, but it turns out that that solution is still flawed, as it allows for passing in values in the wrong order, as well as allowing multiples of same values being passed. I'm kind of stuck in progressing. Making sure the Object has it's keys defined in order and using Object.Values(IMAGE_CONTRAST_VALUES), would give us the right values, but if someone changes the order of the keys, the error would come back, anyone that can help with defining a type that forces all keys to be used in a specified order?

  • Are the values *all* numbers? What specific order should the values be in? – kelsny Oct 19 '22 at 19:41
  • All the values are numbers, so order doesn't matter. – Martin Hansson Oct 19 '22 at 19:53
  • This seems like a strange thing to want. Somewhere you presumably are going to convert between an array and an object or vice versa, so you'll have to choose an order, right? With a type like `["base", "min", "max"]` or `{base: 0, min: 1, max: 2}` you could do this with a mapped type or something like it. But as it stands you're asking someone to write a "how many elements are in this union" type function and the only ways I know to do that are either evil or unscalable. So I'd rather see a convincing use case before plunging into that. – jcalz Oct 19 '22 at 20:01
  • (Also... is there a reason you're writing `InterfaceToTuple` as a distributive type? Do you need it to distribute over unions? If so could you show the use case? If not could we remove the distribution?) Let me know how to proceed (pls mention @jcalz if you reply to notify me) – jcalz Oct 19 '22 at 20:03
  • @jcalz You're correct about us converting, basically in the codebase we have some code ```const CONTRAST_VALUES = [ IMAGE_CONTRAST_VALUES.min, IMAGE_CONTRAST_VALUES.base, IMAGE_CONTRAST_VALUES.max, ]; ``` Recently a bug was commited where another IMAGE_CONTRAST_VALUES.base was added causing issues, this because the imlicit type is number[], to prevent this in the future, we want to secure it so this array only ever be a tuple with the length of the keys in interface, and preferably is types, – Martin Hansson Oct 19 '22 at 20:45
  • I don't know if i need to distribute that was my initial plan, but i don't think i need a mapped type, as I don't care for the keys, I could type it as a tuple with 3 values, [number, number, number], But if the object changes, that won't be reflected in the type declaration for the tuple that way. – Martin Hansson Oct 19 '22 at 20:54
  • And what breaks if you use a `const` assertion in your definition like `const CONTRAST_VALUES = [......] as const`? Then the length will be known. This feels like an XY problem, especially because length is a poor substitute for proper type checking. You wouldn't catch, say `[I.min, I.min, I.max]`, or the values being in an order you don't expect. Anyway, I'm happy to write the "count the elements in this union" type function, I just doubt it's the right tool for the job. (Remember, @jcalz or I'm not notified) – jcalz Oct 19 '22 at 20:55
  • @jcalz - This is correct, yes? Hmm yes didn't even think about the fact that values being out of order would still possibly be an issue, or that passing multiple wrong ones, in a tuple with length 3. const CONTRAST_VALUES = [...] as const, would still let the same error pass as was done, i.e put another value inside the array. How would you suggest creating a type that checks the order of the type and that all the correct keys are passed, Record, and Object.Keys() over the object with the Record? – Martin Hansson Oct 19 '22 at 21:05
  • I'm not sure but I think unless you [edit] the question with that specific use case spelled out, then it's out of scope. I can give you [this solution](https://tsplay.dev/WoAb9N) which gives you a tuple in some random order that [you can't count on](https://tsplay.dev/w2zkYw), as described [here](https://stackoverflow.com/a/55128956/2887218). If that suffices I'll write up an answer. Let me know either way. – jcalz Oct 19 '22 at 21:11
  • @jcalz - Reading the answer you posted, it seems like it indeed is a really bad idea to move forward with converting the tuple to a union, and it won't solve the issues you pointed out above with ordering, and same key being used multiple times, I've edited this post to for this new scope instead. – Martin Hansson Oct 19 '22 at 21:35
  • Maybe you want something like [this approach](https://tsplay.dev/m36EjN) where you specify a tuple of keys and the compiler tries to verify that the tuple is exhaustive (not missing anything) and exclusive (no duplicates), and then maps those keys to the values. If that works for you I will write an answer explaining. (@jcalz to let me know) – jcalz Oct 20 '22 at 02:44
  • @jcalz - Looked through the TS Playground and that seems indeed like it would be a very clean solution, I like the exhaustiveness aspect of it making sure all values are included and that nothing is duplicated, I guess enforcing the order isn't a possibility, but that's fine. Would love an more detailed explanation of how this works that I can show the team and share knowledge around. – Martin Hansson Oct 21 '22 at 12:10
  • I'm not sure how you'd like to enforce ordering, or where it should be specified. After all, the ordering can't dynamically keep up if the interface is changed. I suppose you could just modify the existing approach like [this](https://tsplay.dev/w8xVpw), but now someone has to maintain the ordering tuple. And once you have an ordering tuple you might as well refactor to something like [this](https://tsplay.dev/WPpBLW) which just checks that tuple for consistency with your interface. Let me know how you'd like me to proceed (say @jcalz to summon me) – jcalz Oct 21 '22 at 13:27
  • @jcalz Having the order be specified in an tuple as shown in the examples is totally valid in our use case, and we don't want our ordering to be dynamic. This would catch any of the bugs that actually occurred, and adding a key would force us to revisit the ordering tuple which is a good thing. The way to use Exclude and then using extends never to validate is super interesting, never seen it like that before. I think i understand how you apply missing and extra by comparing both the unions with eachother, but would you mind explaining some of the other stuff such as Dupes, and Keymap typings? – Martin Hansson Oct 22 '22 at 16:10
  • @jcalz - For some reason I don't see an error in the last TS Playground link, however copying the code to try it out in my local environment gives an error that `Type 'K[I]' cannot be used to index type 'T'`- Same error shows in TS Playground if opening the Typescript AST viewer, is this intended? It still shows up as [number, number, number] and the validation works perfectly. – Martin Hansson Oct 22 '22 at 16:13
  • I will write up the answer using [this](https://tsplay.dev/WPpBLW) as the basis for it and explain everything (when I get a chance, that is). The error you mention is from TS4.6 and earlier, and was fixed in TS4.7 via [ms/TS#48837](https://github.com/microsoft/TypeScript/pull/48837). You're likely using an older version of TypeScript; I'd recommend updating and trying again. Generally speaking it's hard to support all versions of TS at once, and the language changes rapidly enough that I usually target the most recently released version, which right now is TS4.8. – jcalz Oct 22 '22 at 16:22
  • @jcalz - That's what i thought, So i updated the typescript version to 4.8.4, but the problem still remains, opening the TS Playground with 4.8 in AST Viewer shows the error aswell. I copied your code to a [Codesandbox](https://codesandbox.io/s/typescript-playground-export-forked-xxgcis) with version 4.8.4, it shows there aswell. – Martin Hansson Oct 22 '22 at 16:54
  • Codesandbox does not update the version of TS the editor uses based on the dependency you set. You can test this yourself by [picking any feature released in, say, TS4.8](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-8.html#unconstrained-generics-no-longer-assignable-to-), and [checking that you can't use it with codesandbox](https://codesandbox.io/s/typescript-playground-export-forked-3kezxb). I'm assuming the same thing is happening with AST Viewer. Does that address the issue or is there still some confusion here? – jcalz Oct 22 '22 at 21:59
  • @jcalz okay I totally see what you are referring to, turns out the TS version in our repo is not the same as the TS version running in VSCode, it works exactly at you pointed out, thx! :) – Martin Hansson Oct 24 '22 at 19:11
  • So now can I write up the answer? Nothing is blocking it? – jcalz Oct 24 '22 at 19:14
  • @jcalz - I'd love if you'd write up an answer, nothing is blocking it. :) – Martin Hansson Oct 26 '22 at 07:12

0 Answers0