New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Referentially Transparent type inference #27502

Open
danvk opened this Issue Oct 2, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@danvk
Copy link

danvk commented Oct 2, 2018

Search Terms

Suggestion

Depending on the context, TypeScript will infer some types in slightly different ways:

  • "foo" can be inferred as either the string literal type "foo" or string.
  • [1, 2] can be inferred as either the tuple type [number, number] or as number[].

One consequence of this is that referential transparency is sometimes broken. See examples below. (Disclaimer: I'm not entirely sure whether I'm using this term correctly!)

I believe this can be a real source of confusion for JavaScript developers coming to TypeScript, since it flags correct code as incorrect and generates errors that require understanding of some advanced features of TypeScript (e.g. string literal types). The solution often involves digging around through type declaration files with no guarantee of success (see also below).

One idea for fixing this would be to base type inference on how a value is used later on. But I assume that has many downsides. Mainly I wanted to raise this issue with the TypeScript team and hear if you consider this a problem or WAI or just impossible to get right.

Examples

Simple example

In plain-old JavaScript, extracting a constant is always OK:

doSomething(value);
// -->
const v = value;
doSomething(v);

This is sometimes OK in TypeScript, sometimes not. Here's an example where it's not:

function panTo(latLng: [number, number]) { /* ... */ }

panTo([10, 20]);  // ok

const x = [10, 20];
panTo(x);  // error:

// Argument of type 'number[]' is not assignable to parameter of type '[number, number]'.
//  Property '0' is missing in type 'number[]'.

Real world example

Real world examples of this often come up with third-party type declarations. For example, in react-mapbox-gl you specify the initial center, zoom, pitch and bearing of the map as tuples:

<Map
  center={[-74, 40.7]}
  zoom={[14.5]}
  pitch={[45]}
  bearing={[-17.6]}
  }>
    { /* ... */ }
</Map>

If you try to factor this out into a constant to avoid magic numbers you get an error:

const INITIAL_VIEW = {
  center: [-74, 40.7],
  zoom: [14.5],
  pitch: [45],
  bearing: [-17.6],
};

return (
  <Map {...INITIAL_VIEW} }>
    { /* ... */ }
  </Map>
);  // error!

The error is a bit of a mouthful:

Type '{ children: Element[]; onClick: () => void; center: number[]; zoom: number[]; pitch: number[]; bearing: number[]; style: string; containerStyle: { height: string; width: string; }; }' is not assignable to type 'Partial<Pick<Readonly<{ children?: ReactNode; }> & Readonly<Props & Events>, "zoom" | "center" | "bearing" | "pitch" | "movingMethod" | "onStyleLoad">>'.
  Types of property 'zoom' are incompatible.
    Type 'number[]' is not assignable to type '[number]'.
      Property '0' is missing in type 'number[]'.

The only solution I'm aware of is to dig through the internals of the type declarations until you find the correct type:

import {Props as MapProps} from 'react-mapbox-gl/lib/map';

const INITIAL_VIEW: Partial<MapProps> = {
  center: [-74, 40.7],
  zoom: [14.5],
  pitch: [45],
  bearing: [-17.6],
};

return (
  <Map {...INITIAL_VIEW} }>
    { /* ... */ }
  </Map>
);  // ok

And this assumes that the relevant type is exported. If not you may have to just give up and throw in an as any.

Both of these examples involved tuples being inferred as arrays, but the same thing can happen with string literals being inferred as string.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@RyanCavanaugh

This comment has been minimized.

Copy link
Member

RyanCavanaugh commented Oct 2, 2018

We're discussing this quite a lot recently; see some comments in #27486. We don't have any ideas at the moment other than the open issues around explicitly denoting literal types.

The real problem here is that we don't have a notion of tracking side effects throughout the type system. Consider something like this:

function fn(tup: [string, number]) {
  tup[0].substr(1);
  tup[1].toFixed();
}

const arr = ["foo", 10];

const m = arr.MYSTERY();

fn(arr);

If MYSTERY is slice, then this code is OK. If MYSTERY is reverse, then this code crashes. But we really don't know the difference between the two, nor do we have any machinery for tracking behavior if the call to MYSTERY is in another function.

Another case is like this:

function fn(obj: { name: "fred" | "bob" }) { /* ... */ }

const obj = { name: "fred" };

// CASE 1
fn.name = "jack";

// CASE 2
fn(obj);

We would like to say that CASE 1 is valid code because this is extremely idiomatic JavaScript. And in the absence of CASE 1, we'd like to say CASE 2 is valid code, because it plainly is. And if both are present, then there's an error. But we would really like to say that the rules for reading and the rules for writing are the same, because in most positions (i.e. function calls) it's ambiguous whether a property is being used for reading or writing.

@danvk

This comment has been minimized.

Copy link
Author

danvk commented Oct 2, 2018

Thanks @RyanCavanaugh. A simpler way to declare that a value is "deeply const" would address the concern around difficulty factoring out a constant.

I was a bit surprised that I couldn't throw a readonly in somewhere to change the inferred types, e.g.

const obj = { readonly name: "fred" };

or maybe even something like (in my example above)

const INITIAL_VIEW: readonly = {
  center: [-74, 40.7],
  zoom: [14.5],
  pitch: [45],
  bearing: [-17.6],
};

Related issues linked to from the meeting notes: #16656 and #10195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment