Skip to content

Improve enumToMap typings#278

Merged
ShogunPanda merged 5 commits into
nodejs:mainfrom
Uzlopak:improve-enumToMap
Jan 30, 2024
Merged

Improve enumToMap typings#278
ShogunPanda merged 5 commits into
nodejs:mainfrom
Uzlopak:improve-enumToMap

Conversation

@Uzlopak

@Uzlopak Uzlopak commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

This improves the typings and also removes one typing issue reported by biome.

the only real runtime change is replacing the regex with a direct property access check.

@Uzlopak

Uzlopak commented Jan 30, 2024

Copy link
Copy Markdown
Contributor Author

remaining linting issues will be fixed in #277

@Uzlopak

Uzlopak commented Jan 30, 2024

Copy link
Copy Markdown
Contributor Author

@ShogunPanda
It seems that node test runner has some issue under windows (from the mocha removal PR). Working on it to get it fixed.

@ShogunPanda

Copy link
Copy Markdown
Contributor

Thanks!

About this PR, I think we should instead get rid of enums completely (which I find very evil in TypeScript), rather than try to fix a mapping function.
I will work on this in like 30 min. WDYT?

@Uzlopak

Uzlopak commented Jan 30, 2024

Copy link
Copy Markdown
Contributor Author

I personally dislike named enums in typescript too, as the transpiled javascript result is very much unreadable and imho garbage. But I was hesitating, because theoretically replacing enums can be done easily by using also interfaces and basically duplicating the structure, and did not know if you would actually accept it or if it is how you would solve it.

But If you have a better approach, than I have no objection ;).

I just think that we could get the CI green if you merge this PR. Then we have a clean slate and you can make your change on top of it.

I will merge now main into this PR to see the green pipeline :D

@Uzlopak

Uzlopak commented Jan 30, 2024

Copy link
Copy Markdown
Contributor Author

Its green. Yay. :D

@ShogunPanda ShogunPanda merged commit e486f51 into nodejs:main Jan 30, 2024
@Uzlopak Uzlopak deleted the improve-enumToMap branch January 30, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants