reduxjs / redux-devtools

DevTools for Redux with hot reloading, action replay, and customizable UI
http://youtube.com/watch?v=xsSnOQynTHs
MIT License
13.99k stars 1.16k forks source link

react-json-tree NodeType definition #659

Open paztis opened 3 years ago

paztis commented 3 years ago

Currently NodeType is only defined as a String It can be great to have a constants / enum definition to avoid to hardcode their usage in our project, like this:

export enum NodeType {
    Object ='Object',
    Error ='Error',
    WeakMap = 'WeakMap',
    WeakSet = 'WeakSet',
    Array = 'Array',
    Iterable = 'Iterable',
    Map = 'Map',
    Set = 'Set',
    String = 'String',
    Number = 'Number',
    Boolean = 'Boolean',
    Date ='Date',
    Null = 'Null',
    Undefined = 'Undefined',
    Function = 'Function',
    Symbol = 'Symbol',
    Custom = 'Custom'
}

I found these list of names by searching in your source code. even not sure I've found all the cases

Methuselah96 commented 3 years ago

Yeah, I've definitely planned on doing this. I've been focusing on more of a breadth-first approach to working on the packages in this monorepo and so I haven't gotten around to doing this yet. See https://github.com/reduxjs/redux-devtools/issues/530 for my immediate goals.

Feel free to make a PR for this yourself. If you don't make a PR, I'll get to it eventually, but it's not high on the list of priorities for me at the moment.

On a final note, I prefer string constants instead of enums since it's easier to define a subset of them as a new type and they feel closer to actual JS to me, but I'm not completely against an enum if that's how want to do it.

paztis commented 3 years ago

You're true One advantage of the enum is you can ensure you will never pass a value not allowed. But a type alias like type TNodeType = 'String' | 'Number' | ... Is also a correct solution that can be extend and prevent the bad cases

Le mer. 28 oct. 2020 à 16:56, Nathan Bierema notifications@github.com a écrit :

Yeah, I've definitely planned on doing this. I've been focusing on more of a breadth-first approach to working on the packages in this monorepo and so I haven't gotten around to doing this yet. See #530 https://github.com/reduxjs/redux-devtools/issues/530 for my immediate goals.

Feel free to make a PR for this yourself. If you don't make a PR, I'll get to it eventually, but it's not high on the list of priorities for me at the moment.

On a final note, I prefer string constants instead of enums since it's easier to define a subset of them as a new type, but I'm not completely against an enum if that's how want to do it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/reduxjs/redux-devtools/issues/659#issuecomment-718030315, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKMMOEAWMM4H3VPREGSFTSNA5JJANCNFSM4TCMKZNQ .