facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.09k stars 1.85k forks source link

[Question] Use external constant when defining flow literal type #4279

Open fqborges opened 7 years ago

fqborges commented 7 years ago

I would like to use an imported constant from a package in a flow literal type and static check the switch.

Example:

// action/types.js
import { REHYDRATE } from 'redux-persist/constants'

export type FooBar = {
  foo: number,
  bar: string,
};

export type Action
  = { type: 'FETCH_REQUEST', data: FooBar[] }
  | { type: REHYDRATE, payload: any } // <== this do not work
  ;

// reducer.js
import { REHYDRATE } from 'redux-persist/constants'

export default function (state: State = initialState, action: Action) 
    switch (action.type) {
    case 'FETCH_REQUEST': 
        // do something with action.data
    case REHYDRATE: { // <= flow says: uncovered code
        // do something with action.payload
    default:
        return state
    }
}

Related StackOverflow question: https://stackoverflow.com/q/44834435/1715004

TrySound commented 7 years ago

The problem is that types are static and removed in build time, but constant bindings are dynamic and can be computed in way that can't be inferred without runtime. You only can declare that constant as a type like this

export const REHYDRATE = 'REHYDRATE';
export type Rehydrate  = 'REHYDRATE ';
psirenny commented 6 years ago

This is such a frequent use case (at least for me) and I often don't want to colocate my flow type and js constants so it leads to errors.

ezmiller commented 5 years ago

Has anyone thought about this more / discovered a way to work around this problem? It does seem to be quite a challenge given that it's likely an issue with flow not being able to see what's happening at runtime.

fqborges commented 5 years ago

My workaround is ugly as hell, but make things work as expected.

// action/types.js
import { REHYDRATE as REDUX_PRESIST_REHYDRATE } from 'redux-persist/constants'

// workaround for https://github.com/facebook/flow/issues/4279#
// redeclare the constant using the original value from lib
export const REHYDRATE: 'persist/REHYDRATE' = 'persist/REHYDRATE';
// static check it have not changed
if (REHYDRATE !== REDUX_PRESIST_REHYDRATE) {
  throw Error('redux-persist REHYDRATE changed! check code to fix');
}

export type FooBar = {
  foo: number,
  bar: string,
};

export type Action
  = { type: 'FETCH_REQUEST', data: FooBar[] }
  | { type: REHYDRATE, payload: any } // <== this do not work
  ;

// reducer.js
import { REHYDRATE } from 'redux-persist/constants'

export default function (state: State = initialState, action: Action) 
  switch (action.type) {
    case 'FETCH_REQUEST': 
      // do something with action.data
    case REHYDRATE: { // <= flow is happy now!
      // do something with action.payload
    default:
      return state
  }
}
benwiley4000 commented 5 years ago

As I noted in the thread I just closed, this is emerging as a very common pattern in my code:

const CONSTANT_A = 'a';
type t_CONSTANT_A = 'a';

const CONSTANT_B = 'b';
type t_CONSTANT_B = 'b';

const CONSTANT_C = 'c';
type t_CONSTANT_C = 'c';

type a_b_or_c = t_CONSTANT_A | t_CONSTANT_B | t_CONSTANT_C;

The highly reasonable but currently impossible pattern would be:

const CONSTANT_A = 'a';

const CONSTANT_B = 'b';

const CONSTANT_C = 'c';

type a_b_or_c = CONSTANT_A | CONSTANT_B | CONSTANT_C;

I understand in theory the issue @TrySound mentioned:

The problem is that types are static and removed in build time, but constant bindings are dynamic and can be computed in way that can't be inferred without runtime.

It's not at all clear to me why this isn't possible in the case where our const value is defined as a string literal. It's statically knowable what that constant's value will be - it's written right there in the file.

Flow should be able to understand values used as types in cases where the value is 1) const and 2) a primitive literal.

ezmiller commented 5 years ago

@benwiley4000 maybe it is relevant specifically to the case where the constant is imported? Using constants directly seems to work if they are defined in the same file: example

fqborges commented 5 years ago

@benwiley4000 when typing the constants and using typeof on the union it works.

const CONSTANT_A : 'a' = 'a';

const CONSTANT_B : 'b' = 'b';

const CONSTANT_C : 'c' = 'c';

type a_b_or_c = typeof CONSTANT_A | typeof CONSTANT_B | typeof CONSTANT_C;

Example

benwiley4000 commented 5 years ago

@fqborges woah! Didn't realize about that one... thank you. I assumed typeof CONSTANT_A would be string.

@ezmiller hmm, we might not be discussing the same problem after all. I'm talking about using a constant AS the type, not recreating it as a literal. But as @fqborges demonstrated, it's not that hard to get around this.

fqborges commented 5 years ago

@benwiley4000 if you not specify the 'constant' type it will be string.

const CONSTANT_A : 'a' = 'a';
//  this part    |----|   
benwiley4000 commented 5 years ago

Oh... well, that kind of sucks but it's not as bad.

youngzhao-xyz commented 5 years ago

I like @fqborges's solution. A slightly different solution is to define the values as types

type CONST_A_VALUE = 'a'
type CONST_B_VALUE = 'b'
type CONST_C_VALUE = 'c'

const CONSTANT_A: CONST_A_VALUE = 'a'
const CONSTANT_B: CONST_B_VALUE = 'b'
const CONSTANT_C: CONST_C_VALUE = 'c'

type A_B_OR_C = CONST_A_VALUE | CONST_B_VALUE | CONST_C_VALUE

// no flow error
const test1: A_B_OR_C = 'a' 

// has flow error
const test2: A_B_OR_C  = 'e'

Example: link

Related stack overflow question: https://stackoverflow.com/a/42202467/2259286

danielo515 commented 4 years ago

The solution from @fqborges is the closest thing to being DRY.