aspnet / JavaScriptServices

[Archived] This repository has been archived
Apache License 2.0
3.04k stars 519 forks source link

[ReactReduxSpa] no completion for action type string literals? #512

Closed grantbowering closed 7 years ago

grantbowering commented 7 years ago

The redux-typed approach was taken out in 9b048c5, and I'm trying to switch our in-development code base over to the "new" suggested way of doing things...

At first look, the spreading around of the magic-valueish type strings knee-jerks me against it somewhat, and one can't (currently, at least, and there are some technical challenges in adding it that might mean never, certainly not priority) use an imported const string as a string literal type to take that pain away... but using KnownActions, one does get compiler-enforcement that you type correct values, so I could certainly shake that off and get used to it...

The problem is that there's no auto-completion suggestions for these values in the IDE (not VSCode, nor VS2015u3, nor VS2017RC).
Which surprises me, since it knows exactly what the only acceptable values are and highlights any other value immediately as an error, listing the (at least, first several before trailing off) values you could provide... but you still have to type them all the way out yourself, which seems odd, and kind of a drag:

2016-12-13 11_25_40-aspnetcoresample - microsoft visual studio administrator 2016-12-13 11_26_08-aspnetcoresample - microsoft visual studio administrator

I'm going to open a ticket with typescript for this as well, since it seems like a ts intellisense oversight, but I'm wondering, is this the way the generator will probably stay for a while, or are there other thoughts (based on TS2.1 features maybe) that it might shift to in the near future?

MarkPieszak commented 7 years ago

I believe that there won't be intellisense for these since you are within a String(?)

Typically you want to create some const's for these so they're a little more strongly typed. For example:

export const INCREMENT_COUNT : string = 'INCREMENT_COUNT';
export const DECREMENT_COUNT : string = 'DECREMENT_COUNT ';

switch (action.type) {
   case INCREMENT_COUNT:   // ^ notice this uses the const above
   /* ... */

Now when you're using this within your App you can actually import these import { INCREMENT_COUNT } when dispatching events etc as well!

grantbowering commented 7 years ago

Unfortunately no, that's not what's happening here.

Those type properties aren't typed as strings, they're typed specifically to that string; it's a typescript feature called "string literal types". The ts compiler will enforce at compile time that the property can ONLY be that string (or undefined), and any code that would set it to anything else is a compiler error. Hence the error tooltip in the first screenshot.

Here's the full code in question generated by the ReactReduxSpa template, for your reference:

import { Reducer } from 'redux';

export interface CounterState {
    count: number;
}

interface IncrementCountAction { type: 'INCREMENT_COUNT' }
interface DecrementCountAction { type: 'DECREMENT_COUNT' }

export type KnownAction = IncrementCountAction | DecrementCountAction;

export const reducer: Reducer<CounterState> = (state: CounterState, action: KnownAction) => {
    switch (action.type) {
        case 'INCREMENT_CO':
            return { count: state.count + 1 };
        case 'DECREMENT_COUNT':
            return { count: state.count - 1 };
        default:
            const exhaustiveCheck: never = action;
    }

    return state || { count: 0 };
};

Note that these are interfaces, and specify a literal string value where one would otherwise put the keyword string in order to type them. That's how you declare a string literal type. Then KnownAction is declared to be the union type of those two interfaces, and the type of the incoming action parameter, which means that action.type can only be one of those two string values. That's how type checking is enforced in this template and what causes the desirable error above.

Believe me, I wish I could make these consts, but I can't; typescript won't let you declare a string literal type based on a const. It has to be a literal right there on the declaration. Hence the name string literal type, I guess, but yeah, it makes this typescript redux code ugly, and that's sort of my point; if I can't get auto-completion wherever else I'm using them then it makes me feel dirty, even though I know a typo would cause a compiler error :(

grantbowering commented 7 years ago

(Linking Microsoft/TypeScript#12891 I just created that is related to this issue... it immediately got correctly labeled as "Suggestion" and "Domain: Completion Lists", so I wouldn't hold my breath as I would assume that's low-priority :slightly_smiling_face: )

MarkPieszak commented 7 years ago

Ahh I see what you're doing I didn't see the KnownTypes interface you made for action. Yeah I wonder if that will get implemented anytime coming up.

SteveSandersonMS commented 7 years ago

@grantbowering Totally agreed with your analysis. Hopefully the TypeScript team will add autocompetion on typed string literals!

Closing as there's no issue to address in this repo.