Change all project enums to objects with as const #2445
Conversation
|
Thanks for this. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2445 +/- ##
========================================
Coverage 99.94% 99.94%
========================================
Files 329 329
Lines 13188 13311 +123
Branches 1367 1309 -58
========================================
+ Hits 13181 13304 +123
Misses 7 7 ☔ View full report in Codecov by Sentry. |
dolanmiu
force-pushed
the
feat/EnumsToObjects
branch
from
35218d6 to
517ae1b
Compare
| export const DocumentGridType = { | ||
| DEFAULT: "default", | ||
| LINES: "lines", | ||
| LINES_AND_CHARS: "linesAndChars", | ||
| SNAP_TO_CHARS: "snapToChars", | ||
| } as const; |
There was a problem hiding this comment.
having this is beneficial, but it may be challenging to use types in the code base and not having clean types, Additionally using PascalCase for variables is not considered a good practice I would recommend reconsidering the implementation of the types like this:
| export const DocumentGridType = { | |
| DEFAULT: "default", | |
| LINES: "lines", | |
| LINES_AND_CHARS: "linesAndChars", | |
| SNAP_TO_CHARS: "snapToChars", | |
| } as const; | |
| const documentGridType = { | |
| DEFAULT: "default", | |
| LINES: "lines", | |
| LINES_AND_CHARS: "linesAndChars", | |
| SNAP_TO_CHARS: "snapToChars", | |
| } as const; | |
| export type DocumentGridType = (typeof documentGridType)[keyof typeof documentGridType] |
or refer to the TypeScript documentation
| export const DocumentGridType = { | |
| DEFAULT: "default", | |
| LINES: "lines", | |
| LINES_AND_CHARS: "linesAndChars", | |
| SNAP_TO_CHARS: "snapToChars", | |
| } as const; | |
| const ODocumentGridType = { | |
| DEFAULT: "default", | |
| LINES: "lines", | |
| LINES_AND_CHARS: "linesAndChars", | |
| SNAP_TO_CHARS: "snapToChars", | |
| } as const; | |
| export type DocumentGridType = (typeof documentGridType)[keyof typeof documentGridType] |
I can make a pr to fix it
There was a problem hiding this comment.
I've tried to keep naming of enums, otherwise projects after update package version will fall to errors
If talk about pascal, camel, etc. cases, I'd like to use upper case to show their constant state
There was a problem hiding this comment.
If we make a breaking change, then it has to be a v9 update
I'd like to use upper case to show their constant state
Yes UPPER_CASE to show constants sounds nice to me
There was a problem hiding this comment.
I just forked remark-docx which uses this project and was on version 8.2.0 and this was a breaking change. Thanks to @hshoja for providing a solution. I am no TS expert and never saw that spending hours trying to figure it out.
Enums after compile looks so gross and they needs a little more space in comparison with objects. This PR should reduce final bundle size
Official documentation say that we dont need to use enums if we can use objects with as const
So that forced me to do this PR