Skip to content
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

Add support for defining: string - 'a' (string minus string literal type) #12182

Closed
normalser opened this issue Nov 11, 2016 · 6 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@normalser
Copy link

#11587 allows us to do this:

function test(a: 'a' | string) {
    if (a == 'a') { 
        a // <- type: 'a'
    }
}

It would be great if the following could also work:

type Z = {type:'a',a:string}|{type:string,b:string}
function test(a: Z) {
    if (a.type == 'a') { 
        a // <- type: Z but ideally if it could be {type:'a',a:string}
    }
}
@ahejlsberg
Copy link
Member

ahejlsberg commented Nov 11, 2016

The proposed narrowing of Z wouldn't be correct because a { type: string, b: string } could very well have a type property with the value 'a'. The only way it would be correct is if you could somehow express a type string - 'a' (i.e. a string with any value but 'a'), but we don't have that capability.

BTW, #11587 is about narrowing string to a literal type, not about narrowing something like 'a' | string. In general it is not meaningful to have types like 'a' | string because it is really the same as just string and the compiler will reduce it to that the minute it performs subtype reduction.

@normalser normalser changed the title Narrow string and number types in literal equality checks for nested objects Add support for defining: string - 'a' (string minus string literal type) Nov 12, 2016
@normalser
Copy link
Author

normalser commented Nov 12, 2016

@ahejlsberg Thanks for the response.

I wonder if we could say that 'a' should take precedence over string in case of Z, but if not then

string - 'a' would solve our use case - so I renamed that title to reflect this feature - but if you feel it's Out of scope - feel free to close it.

We are trying to migrate Redux actions/reducers to TS and such a catch-all action would be very useful, for example:

interface AddToDo {
  type: 'addToDo'
  data: {
     name: string
     completed: boolean
  }
}
// ... - other actions

interface CatchAll {
  type: string  // `string - 'addToDo'` would help here
  data: any
}

type Actions = AddToDo | CatchAll

function reduce(action: Actions) {
  switch (action.type) {
     case 'addToDo': 
          action // type: Actions -> ideally: AddToDo
     break; 

    /// any other actions that don't have any inteface defined
    case 'someOtherAction':
       action // type: Actions -> ideally: CatchAll, so that action.data is any
    break;
    case 'someOtherAction2':
       action // type: Actions -> ideally: CatchAll, so that action.data is any
    break;
    // ... more actions
  }
}

Thanks

@aluanhaddad
Copy link
Contributor

@wallverb That is somewhat strange because the "any other actions" case doesn't do what it says. It only handles values that are tagged with 'someOtherAction'. If all other values are always so tagged, then using that tag as the literal type of CatchAll.type correctly describes the behavior of reduce.

@normalser
Copy link
Author

@aluanhaddad Sorry if my example was not clear enough - I updated. The thing is that there are hundreds of such redux actions that don't have any type defined for them and only handful of actions (like AddToDo) that do have them.
Having something like this:

type DefinedActionTypes = 'addToDo' | 'removeToDo' // | .....
type UndefinedActionTypes = string - DefinedActions

interface CatchAll {
  type: UndefiendActionTypes
  data: any
}

would allow to achieve that (to narrow to CatchAll), but I understand if that would be Out of Scope / no interest by anyone else

@aluanhaddad
Copy link
Contributor

@wallverb What I mean is that you need to handle that in the default clause. If they actually have no union tag at runtime, you can write

interface CatchAll {
  type: undefined
  data: any
}

and type narrowing will actually work with case (if you are using --strictNullChecks then undefined is incompatible with string).

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2016

seems like a duplicate of #7993

@mhegazy mhegazy closed this as completed Nov 14, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Nov 14, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants